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

Support the "module" field of package.json for client code. #10541

Merged

Conversation

@benjamn
Copy link
Member

benjamn commented May 2, 2019

Like Node.js, Meteor's module system currently pays attention only to the "main" field of package.json files (and also "browser" on the client) when resolving package entry points. This behavior effectively limits Meteor to using the CommonJS version of any npm packages, even though many packages have both a CommonJS and an ECMAScript module (ESM) entry point.

This PR enables support for the "module" field for client code. The implementation required a few small adjustments: in particular, we now compile .js files within node_modules if they are contained by a package that has a "module" entry point (only if the ImportScanner decides the module is actually used), and our module resolution logic now prefers the "module" entry point both during build (resover.js) and at runtime (modules-runtime).

Why is this important? Supporting ESM syntax everywhere is the future of JavaScript, first and foremost. Also, wherever ESM syntax is used, it's important for Meteor to have a chance to compile it, since we have an advanced module compiler called Reify.

Supporting "module" in package.json for server code is not advisable because Node.js will be adopting the "type": "module" convention instead, and in the meantime we need to maintain consistency with Node's module resolution rules, which only currently pay attention to "main".

@benjamn benjamn added this to the Release 1.8.2 milestone May 2, 2019
@benjamn benjamn requested review from hwillson and abernix May 2, 2019
@benjamn benjamn self-assigned this May 2, 2019
@benjamn benjamn force-pushed the support-module-field-of-package.json-on-client branch from c1b3661 to 36c0090 May 2, 2019
@benjamn benjamn force-pushed the release-1.8.2 branch from 6115580 to ff87169 May 2, 2019
@benjamn benjamn force-pushed the support-module-field-of-package.json-on-client branch from 36c0090 to 9c1769f May 2, 2019
@benjamn benjamn force-pushed the release-1.8.2 branch 2 times, most recently from ada62d5 to f4b92a0 May 2, 2019
Supporting "module" in package.json for server code is not advisable
because Node.js will be adopting the "type":"module" convention instead,
and in the meantime we need to maintain consistency with Node's module
resolution rules, which only currently pay attention to "main":
https://medium.com/@nodejs/announcing-a-new-experimental-modules-1be8d2d6c2ff
@benjamn benjamn force-pushed the support-module-field-of-package.json-on-client branch from 9c1769f to c0cc2e3 May 2, 2019
@benjamn benjamn merged commit f166f22 into release-1.8.2 May 2, 2019
19 checks passed
19 checks passed
CLA Author has signed the Meteor CLA.
Details
ci/circleci: Clean Up Your tests passed on CircleCI!
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 1 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 10 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 4 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 5 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 6 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 7 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 8 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 9 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@benjamn benjamn deleted the support-module-field-of-package.json-on-client branch May 2, 2019
benjamn added a commit that referenced this pull request May 3, 2019
When I implemented support for the "module" entry point in package.json
files for client code in #10541, I modified PackageSource#_findSources to
include files found in node_modules that need to be compiled, but my
implementation considered only "local" node_modules directories, like the
one in the application root directory, while neglecting the private
.npm/package/node_modules directories that many Meteor packages have.

This commit includes .npm/**/node_modules when _findSources is scanning a
Meteor package, which should solve issues like #10544, where a Meteor
package imports an npm package that was installed with Npm.depends, and
that npm package has a "module" field in its package.json file, pointing
to an ESM entry point module, but the ESM syntax was not appropriately
compiled, leading to parse errors like "Unexpected token export".

Before lazy compilation was introduced in Meteor 1.7 (#9983), including
the node_modules directories of Meteor packages would likely have been a
big problem for build performance, since there would be that many more
modules to compile. It's still worth making sure this change doesn't
regress build performance for other reasons, but I'm reasonably confident
lazy compilation will save us here, unless there are just too many npm
packages installed via Npm.depends that export ESM modules.
benjamn added a commit that referenced this pull request May 4, 2019
When I implemented support for the "module" entry point in package.json
files for client code in #10541, I modified PackageSource#_findSources to
include files found in node_modules that need to be compiled, but my
implementation considered only "local" node_modules directories, like the
one in the application root directory, while neglecting the private
.npm/package/node_modules directories that many Meteor packages have.

This commit includes .npm/**/node_modules when _findSources is scanning a
Meteor package, which should solve issues like #10544, where a Meteor
package imports an npm package that was installed with Npm.depends, and
that npm package has a "module" field in its package.json file, pointing
to an ESM entry point module, but the ESM syntax was not appropriately
compiled, leading to parse errors like "Unexpected token export".

Before lazy compilation was introduced in Meteor 1.7 (#9983), including
the node_modules directories of Meteor packages would likely have been a
big problem for build performance, since there would be that many more
modules to compile. It's still worth making sure this change doesn't
regress build performance for other reasons, but I'm reasonably confident
lazy compilation will save us here, unless there are just too many npm
packages installed via Npm.depends that export ESM modules.
benjamn added a commit that referenced this pull request May 4, 2019
When I implemented support for the "module" entry point in package.json
files for client code in #10541, I modified PackageSource#_findSources to
include files found in node_modules that need to be compiled, but my
implementation considered only "local" node_modules directories, like the
one in the application root directory, while neglecting the private
.npm/package/node_modules directories that many Meteor packages have.

This commit includes .npm/**/node_modules when _findSources is scanning a
Meteor package, which should solve issues like #10544, where a Meteor
package imports an npm package that was installed with Npm.depends, and
that npm package has a "module" field in its package.json file, pointing
to an ESM entry point module, but the ESM syntax was not appropriately
compiled, leading to parse errors like "Unexpected token export".

Before lazy compilation was introduced in Meteor 1.7 (#9983), including
the node_modules directories of Meteor packages would likely have been a
big problem for build performance, since there would be that many more
modules to compile. It's still worth making sure this change doesn't
regress build performance for other reasons, but I'm reasonably confident
lazy compilation will save us here, unless there are just too many npm
packages installed via Npm.depends that export ESM modules.
benjamn added a commit that referenced this pull request May 4, 2019
When I implemented support for the "module" entry point in package.json
files for client code in #10541, I modified PackageSource#_findSources to
include files found in node_modules that need to be compiled, but my
implementation considered only "local" node_modules directories, like the
one in the application root directory, while neglecting the private
.npm/package/node_modules directories that many Meteor packages have.

This commit includes .npm/**/node_modules when _findSources is scanning a
Meteor package, which should solve issues like #10544, where a Meteor
package imports an npm package that was installed with Npm.depends, and
that npm package has a "module" field in its package.json file, pointing
to an ESM entry point module, but the ESM syntax was not appropriately
compiled, leading to parse errors like "Unexpected token export".

Before lazy compilation was introduced in Meteor 1.7 (#9983), including
the node_modules directories of Meteor packages would likely have been a
big problem for build performance, since there would be that many more
modules to compile. It's still worth making sure this change doesn't
regress build performance for other reasons, but I'm reasonably confident
lazy compilation will save us here, unless there are just too many npm
packages installed via Npm.depends that export ESM modules.
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.