From 4abc45a4b9d4a6e5ad682ec85ea9f5988671392e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 24 Apr 2020 23:08:57 +0200 Subject: [PATCH] module: do not warn when accessing `__esModule` of unfinished exports Since this property access is performed by generated code, and not used for accessing the actual exports of a module (and because transpilers generally define it as the first key of `module.exports` when it *is* present), it should be okay to allow it. Refs: https://github.com/nodejs/node/pull/29935 Fixes: https://github.com/nodejs/node/issues/33046 PR-URL: https://github.com/nodejs/node/pull/33048 Reviewed-By: Guy Bedford Reviewed-By: James M Snell Reviewed-By: Zeyu Yang --- lib/internal/modules/cjs/loader.js | 7 +++++-- test/fixtures/cycles/warning-esm-half-transpiled-a.js | 1 + test/fixtures/cycles/warning-esm-half-transpiled-b.js | 2 ++ test/parallel/test-module-circular-dependency-warning.js | 7 +++++++ 4 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/cycles/warning-esm-half-transpiled-a.js create mode 100644 test/fixtures/cycles/warning-esm-half-transpiled-b.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 96988a6a471aad..f2715d07cb44fb 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -824,13 +824,16 @@ function emitCircularRequireWarning(prop) { // warns when non-existend properties are accessed. const CircularRequirePrototypeWarningProxy = new Proxy({}, { get(target, prop) { - if (prop in target) return target[prop]; + // Allow __esModule access in any case because it is used in the output + // of transpiled code to determine whether something comes from an + // ES module, and is not used as a regular key of `module.exports`. + if (prop in target || prop === '__esModule') return target[prop]; emitCircularRequireWarning(prop); return undefined; }, getOwnPropertyDescriptor(target, prop) { - if (ObjectPrototypeHasOwnProperty(target, prop)) + if (ObjectPrototypeHasOwnProperty(target, prop) || prop === '__esModule') return ObjectGetOwnPropertyDescriptor(target, prop); emitCircularRequireWarning(prop); return undefined; diff --git a/test/fixtures/cycles/warning-esm-half-transpiled-a.js b/test/fixtures/cycles/warning-esm-half-transpiled-a.js new file mode 100644 index 00000000000000..f4c17f9e4adaaf --- /dev/null +++ b/test/fixtures/cycles/warning-esm-half-transpiled-a.js @@ -0,0 +1 @@ +require('./warning-esm-half-transpiled-b.js'); diff --git a/test/fixtures/cycles/warning-esm-half-transpiled-b.js b/test/fixtures/cycles/warning-esm-half-transpiled-b.js new file mode 100644 index 00000000000000..ab31fdbffc1a07 --- /dev/null +++ b/test/fixtures/cycles/warning-esm-half-transpiled-b.js @@ -0,0 +1,2 @@ +const a = require('./warning-esm-half-transpiled-a.js'); +a.__esModule; diff --git a/test/parallel/test-module-circular-dependency-warning.js b/test/parallel/test-module-circular-dependency-warning.js index 10893978829ff5..1fe82c2b0288d5 100644 --- a/test/parallel/test-module-circular-dependency-warning.js +++ b/test/parallel/test-module-circular-dependency-warning.js @@ -31,3 +31,10 @@ assert.strictEqual(Object.getPrototypeOf(classExport).name, 'Parent'); const esmTranspiledExport = require(fixtures.path('cycles', 'warning-esm-transpiled-a.js')); assert.strictEqual(esmTranspiledExport.__esModule, true); + +// If module.exports.__esModule is being accessed but is not present, e.g. +// because only the one of the files is a transpiled ES module, no warning +// should be emitted. +const halfTranspiledExport = + require(fixtures.path('cycles', 'warning-esm-half-transpiled-a.js')); +assert.strictEqual(halfTranspiledExport.__esModule, undefined);