Avoid bailing out with module.useNode() for ESM modules.

On the server, Meteor attempts to avoid bundling node_modules code by
replacing entry point modules with a stub that calls module.useNode() (see
packages/modules-runtime/server.js). This trick allows evaluating server
node_modules natively in Node.js, faithfully preserving all Node-specific
behaviors, such as module.id being an absolute file system path, the
__dirname and __filename variables, the ability to import binary .node
modules, and so on.

However, starting in Node.js 12.16.0 (Meteor 1.9.1+), modules evaluated
natively by Node are considered ECMAScript modules (ESM) if the closest
package.json file has "type": "module" (or has an .mjs file extension).
This poses a problem for the module.useNode() trick, because ESM modules
cannot be imported synchronously using require (which is currently how
module.useNode() works).

To work around this new error, this commit checks package.json for "type":
"module" in ImportScanner#shouldUseNode to determine whether it's safe to
use the module.useNode() trick.

The good news is that ESM modules don't have access to nearly as many
Node.js-specific quirks: no module, require, or exports variables; no
__dirname, no __filename; no ability to import JSON or other non-ESM file
types (at least right now). So it seems somewhat less important for ESM
code (compared to CommonJS code) to bail out into native Node.js execution
using module.useNode(). In other words, bundling server code should not
affect its execution in nearly as many cases, if that code is ESM rather
than legacy CommonJS.

If this good news turns out to be overly optimistic, we can consider using
a different kind of bailout stub that's capable of importing ESM using
dynamic import(). For now, making sure we avoid bailing out for ESM code
like @babel/runtime/helpers/esm/* is the priority.
This commit is contained in:
Ben Newman
2020-02-19 12:26:15 -05:00
parent 290e084cab
commit 87bc54fb9f
2 changed files with 48 additions and 6 deletions

View File

@@ -71,6 +71,10 @@ Module.prototype.useNode = function () {
return false;
}
// See tools/static-assets/server/npm-require.js for the implementation
// of npmRequire. Note that this strategy fails when importing ESM
// modules (typically, a .js file in a package with "type": "module" in
// its package.json), as of Node 12.16.0 (Meteor 1.9.1).
this.exports = npmRequire(this.id);
return true;

View File

@@ -39,6 +39,7 @@ import {
optimisticStatOrNull,
optimisticLStatOrNull,
optimisticHashOrNull,
optimisticLookupPackageJsonArray,
} from "../fs/optimistic";
import { wrap } from "optimism";
@@ -1296,7 +1297,7 @@ export default class ImportScanner {
// raw version found in node_modules. See also:
// https://github.com/meteor/meteor-feature-requests/issues/6
} else if (this.shouldUseNode(absModuleId)) {
} else if (this.shouldUseNode(absModuleId, absPath)) {
// On the server, modules in node_modules directories will be
// handled natively by Node, so we just need to generate a stub
// module that calls module.useNode(), rather than calling
@@ -1340,7 +1341,7 @@ export default class ImportScanner {
// Similar to logic in Module.prototype.useNode as defined in
// packages/modules-runtime/server.js. Introduced to fix issue #10122.
private shouldUseNode(absModuleId: string) {
private shouldUseNode(absModuleId: string, absPath: string) {
if (this.isWeb()) {
// Node should never be used in a browser, obviously.
return false;
@@ -1360,10 +1361,47 @@ export default class ImportScanner {
start += 2;
}
// If the remaining parts include node_modules, then this is a module
// that was installed by npm, and it should be evaluated by Node on
// the server.
return parts.indexOf("node_modules", start) >= 0;
// If the remaining parts do not include node_modules, then this
// module was not installed by npm, so we should not try to evaluate
// it natively in Node on the server.
if (parts.indexOf("node_modules", start) < 0) {
return false;
}
// Below this point, we know we're dealing with a module in
// node_modules, which means we should try to use module.useNode() to
// evaluate the module natively in Node, except if the module is an
// ESM module, because then the module cannot be imported using
// require (as of Node 12.16.0), so module.useNode() will not work.
const dotExt = pathExtname(absPath).toLowerCase();
if (dotExt === ".mjs") {
// Although few npm packages actually use .mjs, Node will always
// interpret these files as ESM modules, so we can return early.
return false;
}
if (dotExt === ".json") {
// There's no benefit to using Node to evaluate JSON modules, since
// there's nothing Node-specific about the parsing of JSON.
return false;
}
if (dotExt === ".js") {
const relDir = pathRelative(this.sourceRoot, pathDirname(absPath));
const pkgJsonArray =
optimisticLookupPackageJsonArray(this.sourceRoot, relDir);
// Setting "type":"module" in package.json makes Node treat .js
// files within the package as ESM modules.
if (pkgJsonArray.some(pkgJson => pkgJson?.type === "module")) {
return false;
}
}
// Everything else (.node, .wasm, whatever) needs to be handled
// natively by Node.
return true;
}
// Returns an absolute module identifier indicating where to install the