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

Don't transform all modules to commonjs #23

Merged
merged 1 commit into from
Jun 30, 2021
Merged

Conversation

juliusknorr
Copy link
Contributor

Enforcing modules transformation to commonjs does block webpack from using code splitting.

From https://babeljs.io/docs/en/babel-preset-env:

Setting this to false will preserve ES modules. Use this only if you intend to ship native ES Modules to browsers. If you are using a bundler with Babel, the default modules: "auto" is always preferred.
modules: "auto"

By default @babel/preset-env uses caller data to determine whether ES modules and module features (e.g. import()) should be transformed. Generally caller data will be specified in the bundler plugins (e.g. babel-loader, @rollup/plugin-babel) and thus it is not recommended to pass caller data yourself -- The passed caller may overwrite the one from bundler plugins and in the future you may get suboptimal results if bundlers supports new module features.

Encountered in nextcloud/text#1659 where the dynamic imports with a /* webpackChunkName */ comment no longer created chunks with the shared configs, when testing i also noticed that this is the case for the contacts app where the moment chunks are no longer built.

@skjnldsv Was there any specific reason this was explicitly set to commonjs?

@juliusknorr juliusknorr requested review from artonge and skjnldsv June 30, 2021 06:00
@skjnldsv
Copy link
Contributor

@skjnldsv Was there any specific reason this was explicitly set to commonjs?

I honestly forgot, that was needed by some other apps iirc.
Like talk 🤔

@skjnldsv
Copy link
Contributor

Should be a major then 🚀

@skjnldsv skjnldsv merged commit be4e837 into master Jun 30, 2021
@skjnldsv skjnldsv deleted the bugfix/commonjs branch June 30, 2021 14:51
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.

3 participants