Skip to content

Webpack + browser target requires additional transpilation #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mattvperry opened this issue Aug 24, 2017 · 5 comments
Closed

Webpack + browser target requires additional transpilation #2

mattvperry opened this issue Aug 24, 2017 · 5 comments

Comments

@mattvperry
Copy link

mattvperry commented Aug 24, 2017

I believe this issue applies to a couple of your other typescript-fsa-* projects but I found this one to be the most worrisome so I am filing the bug here.

The issue is that you are shipping ES6 code in the es/ part of this module. By default, webpack looks at the package.json field 'module' first so if anyone using webpack imports this then they are going to get the ES6 generator syntax whether they want it or not. You also ship an ES5 target in lib/ but that is in your 'main' field which webpack won't look at since you have a 'module'.

There are a couple of workarounds for anyone encountering this:

  1. Change webpack's resolve.mainFields array to have 'main' first (note: this will affect all imports)
  2. Add the below to your webpack config:
alias: {
    // Workaround for this specific library because it ships an ES6 module as its default export
    // but also ships an ES5 module under /lib/ which can be polyfilled
    "typescript-fsa-redux-saga": path.resolve(__dirname, "node_modules/typescript-fsa-redux-saga/lib/index.js")
}
  1. Make babel transpile the node_module in your webpack config. This seems like a code smell though since you shouldn't need to transpile your dependencies. This is especially troublesome if you werent already using babel for anything.

There are several fixes that I can think of:

  1. Don't ship the es/ module and only ship lib/ (You would still require babel-polyfill as a dependency because of regenerator)
  2. Use TSC to actually transpile this all the way down to ES5 or even ES3. This would help people who don't want to get babel involved at all if they dont need to. (note: an ES3 target would help typescript-fsa since you use Object.assign there which would require a polyfill)

This lib works fine as-is in Chrome because it seems to support the generator syntax. This really becomes an issue when you need to work with IE

@aikoven
Copy link
Owner

aikoven commented Sep 4, 2017

Thanks, that makes a lot of sense.

TypeScript compiles generators to ES3 quite well, although it includes its own generator runtime which is quite big. Multiply this by the number of files with generators in your project and you'll get a huge overhead.

This could be solved by using tsconfig field importHelpers, which makes it possible to use a single generator runtime from tslib.
The downside of this is that we'd include a whole tslib which is much larger than just the generator runtime. However, I would recommend using importHelpers for everyone, in which case tslib would be reused across the whole project. Also, this problem will vanish once tslib becomes tree-shakeable.

So I think I'll set the target to ES3 and include tslib as a dependency. What do you think?

Considering Object.assign: it seems that TS doesn't transpile it at all: microsoft/TypeScript#3429

@mattvperry
Copy link
Author

mattvperry commented Sep 4, 2017

Yeah importHelpers and tslib are awesome for this. You should be able to declare tslib as a peer dependency if you go along that route maybe?

As far as Object.assign goes I am surprised that TS doesn't transpile it. Oh well, it was less of an issue than the ES6 generator syntax thing anyway.

@aikoven
Copy link
Owner

aikoven commented Sep 4, 2017

Unfortunately, I don't see a way to make tslib opt-in: it's either a dependency of the library or the generator runtime is inlined in the distributed code.

@aikoven
Copy link
Owner

aikoven commented Sep 15, 2017

Just released v1.0.3.

@mattvperry
Copy link
Author

Awesome, looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants