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

fix: do not exclude node modules from babel #194

Closed
wants to merge 1 commit into from

Conversation

brandonocasey
Copy link
Contributor

@brandonocasey brandonocasey commented Jul 13, 2018

In order to get many of the size benefits from rollup/es6 code we want to use es6 versions of sub modules. Right now that is what we do, but it causes syntax errors (especially on ie 11) since anything in node_modules won't be converted to es5 and will remain es6.

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that Babel is still not being run for ES6 modules, so I think this makes sense.

@gkatsev
Copy link
Member

gkatsev commented Jul 13, 2018

do we have dependencies that ship es6 code hidden in es modules?

@brandonocasey
Copy link
Contributor Author

brandonocasey commented Jul 13, 2018

Any plugin that uses the generator that depends on another plugin that uses the generator will have this problem, because rollup prioritizes the module entry in package.json and uses that. Previously this wasn't a problem because our module/es entry was actually being babeled down to es5 which kind of defeated the point of having an es/module export.

@gkatsev
Copy link
Member

gkatsev commented Jul 13, 2018

Is the .es.js output using es6 code? The only non es5 code it should have is import statements (es modules stuff)

@brandonocasey
Copy link
Contributor Author

shouldn't the es module basically be the source code all bundled together without any transpiration to es5? For instance a class stays a class.

@gkatsev
Copy link
Member

gkatsev commented Jul 13, 2018

no, the pkg.module should basically just be es module stuff, everything else should be compiled down to work on our standard browser support, so, IE11+ etc. We shouldn't be forcing users of our module to run babel on our stuff.

@brandonocasey
Copy link
Contributor Author

After further discussion it appears that the es dist should be babeled and I was wrong. https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

Closing this in favor of #196 which brings that functionality back.

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

Successfully merging this pull request may close these issues.

None yet

3 participants