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

Prefer "main" field of package.json over "module" in legacy bundle. #10765

Merged
merged 3 commits into from Nov 8, 2019

Conversation

@benjamn
Copy link
Member

benjamn commented Nov 8, 2019

Before Meteor 1.8.2, all bundles avoided using the "module" field of package.json, in favor of either the "main" field or (when building/running a client-side bundle) the "browser" field.

During the development of Meteor 1.8.2, we have tried several different strategies to begin using the "module" field when possible, before falling back to the "main" field, which should unlock future optimizations like import/export-based tree shaking.

While everything appeared to be working from a logical standpoint, the practical reality is that many npm packages ship dangerously modern syntax like const and arrow functions in the code identified by their "module" fields, not just import and export. This is regrettable, since it means the legacy bundle cannot simply include these npm packages' "module" code without first recompiling it down to ES5. In other words, our decision to prefer "module" over "main" would have made it necessary to add a lot more npm packages to the meteor.nodeModules.recompile list, as @smeijer found in #10658 (comment).

This PR reverses the precedence of "module" and "main" for legacy bundles only, so that legacy client builds will go back to behaving the way they behaved in Meteor 1.8.1. This may limit the impact of tree shaking for the legacy bundle in the future, but that's acceptable because it's more important for the legacy bundle to work correctly (without a lot of manual reconfiguration) than to be as small as possible.

Thanks to @smeijer for this idea! #10658 (comment)

benjamn added 2 commits Nov 8, 2019
#10658 (comment)
#10658 (comment)

As usual, changes to module resolution logic need to happen in parallel in
tools/isobuild/resolver.ts and in packages/modules-runtime. However,
thanks to the modern/legacy system, it's easy to make the modules-runtime
package behave exactly the way(s) we want in the server, modern client,
and legacy client bundles.
@benjamn benjamn added this to the Release 1.8.2 milestone Nov 8, 2019
@benjamn benjamn self-assigned this Nov 8, 2019
@benjamn benjamn merged commit e4597b1 into release-1.8.2 Nov 8, 2019
10 of 18 checks passed
10 of 18 checks passed
ci/circleci: Test Group 1 CircleCI is running your tests
Details
ci/circleci: Test Group 10 CircleCI is running your tests
Details
ci/circleci: Test Group 4 CircleCI is running your tests
Details
ci/circleci: Test Group 5 CircleCI is running your tests
Details
ci/circleci: Test Group 6 CircleCI is running your tests
Details
ci/circleci: Test Group 7 CircleCI is running your tests
Details
ci/circleci: Test Group 9 CircleCI is running your tests
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
ci/circleci: Docs Your tests passed on CircleCI!
Details
ci/circleci: Get Ready Your tests passed on CircleCI!
Details
ci/circleci: Isolated Tests Your tests passed on CircleCI!
Details
ci/circleci: Test Group 0 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 2 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 3 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 8 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@benjamn benjamn deleted the prefer-main-over-module-in-legacy-bundle branch Nov 8, 2019
@benjamn benjamn restored the prefer-main-over-module-in-legacy-bundle branch Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.