-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat: Add support for ECMAScript modules #69
Conversation
Starting from v. 5, file-loader switch to esModule by default
babel-core is required for transpilation, while the babel-plugin-add-module-exports allows impor of transpiled modules without using the .default property.
The reported Syntax error is slighly different when coming from babel
After looking at the tests failing on node 6, I tried to find out what was wrong. So, here's the problem. In the .babelrc of this project it's loaded the Since in this project both package.json and package-lock.json refer to version 5.0.1 of that plugin, everything should work fine since we're bound to that version. However, the npm version 3 that's coming with node 6 does not honor the package-lock.json file, and installs the latest version matching the semantic versioning of package.json, that is 5.2.0. I can think of 2 possible fixes.
By the way, this should probably be done in a separate PR. Let me know what you think |
...or drop Node 6. Is this still being used? Isn't it EOL? |
@carsonreinke I also think that support for v6 could be dropped. However, I'm not sure this PR will be reviewed soon. It's been almost a month since I've opened it, and haven't received any feedback. Do you know someone to mention who could help us in moving this forward? |
Aligns to peerigon master
@jannikkeye @jhnns |
@cybercase sorry for not getting back to you, for sure dropping Node 6. My only thought about this too, this puts a dependency on Babel 6 for projects that use extract-loader. What if I wanted to use Babel 7 in my project and use extract-loader? I really don't see a way around that, unless some sort of peer dependency and Babel version check nonsense. |
@carsonreinke as I said in the first comment of this PR, I agree with you. I liked the approach of #26 that doesn't depend upon any babel version at all. However, following the discussion in that same PR, I read that the maintainer has a different point of view... that's why I choose to use babel in this PR. |
@cybercase Thanks ❤️ I will review this in this week. |
Awesome, thank you 👍 I decided to release it as major version bump because the usage of Babel might introduce unwanted/unknown side-effects in some project setups. I don't think that it's a problem but let's better be safe than sorry :) |
# [5.0.0](v4.0.3...v5.0.0) (2020-03-05) ### Features * Add support for ECMAScript modules ([#69](#69)) ([6034f23](6034f23)) ### BREAKING CHANGES * The extract-loader now uses Babel to transpile the bundle code for the current Node version. This is required in order to support the new file-loader which produces ECMAScript modules. This *should not* be a breaking change but since Babel is also known to be kind of brittle in regards of configuration we cannot guarantee that there might be unknown/unwanted side-effects in your project setup.
🎉 This PR is included in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Following the discussion #67 I've made some changes to add transpilation of es modules.
This should solve the issue with the new default behaviour of file-loader v.5 of enabling esModules by default.
Looking at how file-loader is implemented here, and agreeing with @carsonreinke, I would have opted for a simpler fix, like the one you received in #26 .
However, since you've expressed the preference of using babel to avoid risks, I've followed your preference.
Let me know if you're ok with this PR or if you need me to make further changes.