Skip to content
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

jsnext:main / module build #25

Closed
screendriver opened this issue Feb 25, 2017 · 7 comments
Closed

jsnext:main / module build #25

screendriver opened this issue Feb 25, 2017 · 7 comments

Comments

@screendriver
Copy link

At the moment jsnext:main respectively module points to src/index.js. This leads to the situation where the consuming package / application needs to transpile unfetch as well in their build because older browsers don't understand arrow functions and so on.

jsnext:main or module code should always already be transpiled. The only difference is that they use ES6 module syntax instead of commonjs function calls.

See also webpack/webpack#2902 (comment) and pkg.module in rollup ☺️

@developit
Copy link
Owner

Hmm, so I very strongly disagree about this: ES Modules is part of ES2015. It doesn't make much sense to support only the module semantics of that spec but nothing else. People should be traveling spiking node_modules, or transpiling their whole bundle as a step like you would use Uglify.

That said, if it's an issue I think we should just remove the modules / jsnext entry - there are no compile time optimizations that will affect unfetch, since it exports a single function.

@screendriver
Copy link
Author

transpiling their whole bundle as a step like you would use Uglify.

That's the reason why I discovered this. I added unfetch 2 to my project that uses webpack as build system and UglifyJS complains about a syntax error. This syntax error was because it can't deal with arrow functions. The solution would be that I modify my babel-loader oder ts-loader to not exclude /node_modules/ and make an exception for unfetch.

I read a lot out there about jsnext:main and module and every package / bundler that I know recommends to exclude node_modules in transpilation because it can lead to strange behaviours because it forces the consuming package to use exactly the tools that the referenced library use. For example if unfetch would use decorators or async and await my package has to add this features in my build as well. Things getting more worse if the library uses a .babelrc that will be published to npm. Then the babel parser from the consuming project parses this .babelrc as well and overrides the own settings.

The whole ecosystem is a little bit torn and confused about this whole stuff (for instance the whole .mjs thing in Node)

@developit
Copy link
Owner

Agreed about mismatched transpile assumptions, and you're right this isn't a polar issue - sorry if my reply seemed a little quick on the draw there haha. I'm fine if there's a nice way to output esm from rollup (perhaps this technique?), but I do think it'll actually end up being larger for people who use the modules entry - uglify needs to run on this codebase to compress it down properly.

@tunnckoCore
Copy link
Collaborator

are u using the webpack.uglify or? i dont believe it not work with es6, if it is true... damn webpack uglysystem.

it is true, Jason, that it not make sense to just let the esmodules syntax and transpile evrything other.

these fields are meant to be in es2015 syntax and above, no matter what feature.

@screendriver
Copy link
Author

screendriver commented Feb 26, 2017

Yes I am using the webpack built in UglifyJS. Uglify does not work with 2015. You can reproduce it very easy:

Create a file foo.js:

const foo = () => console.log('bar');

Run UglifyJS:

❯ uglifyjs foo.js 
Parse error at foo.js:1,13
SyntaxError: Unexpected token:

Also there is a big issue about tree shaking and UglifyJS in webpack: webpack/webpack#2867 and one in UglifyJS regarding harmony mishoo/UglifyJS#448

it is true, Jason, that it not make sense to just let the esmodules syntax and transpile evrything other.
these fields are meant to be in es2015 syntax and above, no matter what feature.

That's not really true as I said before. jsnext:main and module should only indicate that the package is in ES2015 and not in CommonJS module syntax. It does not indicate the language level. For that you could use the engines entry in your package.json.

Imagine you write a really large application with a lot of dependencies and all that dependencies are not "ready to use" transpiled. You would end up in a build system for your application that would be hell to configure and maintain (you have to look in every single dependency what language features it use). Not to mention the horrible build time. If you are interested you can read about it here jsforum/jsforum#5

@developit thank you for the very quick 2.1.0! 😎 It now behaves as expected and looks good for me 👍

@developit
Copy link
Owner

@screendriver I maintain such an application as my day job, 30+ repos of ES2015. We transpile everything and it works great.

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Feb 26, 2017

You the hell don't need to know what your deps use. 🚀 In anyway all hippie funky guys nowadays use 50+ devDeps with dozens of Babel-ish presets and shits. Not to mention that the install time is awful even with 1Gb network ; ). So I don't see any problems.

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

3 participants