diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 413fcd27703e3e..ece30c3864d6a7 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -22,7 +22,7 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { debug = fn; }); -const { ModuleWrap, kEvaluated } = internalBinding('module_wrap'); +const { ModuleWrap, kInstantiated } = internalBinding('module_wrap'); const { privateSymbols: { entry_point_module_private_symbol, @@ -354,10 +354,26 @@ class ModuleJobSync extends ModuleJobBase { } async run() { + // This path is hit by a require'd module that is imported again. const status = this.module.getStatus(); - assert(status === kEvaluated, - `A require()-d module that is imported again must be evaluated. Status = ${status}`); - return { __proto__: null, module: this.module }; + if (status > kInstantiated) { + if (this.evaluationPromise) { + await this.evaluationPromise; + } + return { __proto__: null, module: this.module }; + } else if (status === kInstantiated) { + // The evaluation may have been canceled because instantiateSync() detected TLA first. + // But when it is imported again, it's fine to re-evaluate it asynchronously. + const timeout = -1; + const breakOnSigint = false; + this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); + await this.evaluationPromise; + this.evaluationPromise = undefined; + return { __proto__: null, module: this.module }; + } + + assert.fail('Unexpected status of a module that is imported again after being required. ' + + `Status = ${status}`); } runSync() { diff --git a/test/es-module/test-require-module-retry-import-errored.js b/test/es-module/test-require-module-retry-import-errored.js new file mode 100644 index 00000000000000..4736087d2624a5 --- /dev/null +++ b/test/es-module/test-require-module-retry-import-errored.js @@ -0,0 +1,35 @@ +// This tests that after failing to require an ESM that contains TLA, +// retrying with import() still works, and produces consistent results. +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { exportedReject } = require('../fixtures/es-modules/tla/export-promise.mjs'); + +assert.throws(() => { + require('../fixtures/es-modules/tla/await-export-promise.mjs'); +}, { + code: 'ERR_REQUIRE_ASYNC_MODULE' +}); + +const interval = setInterval(() => {}, 1000); // Keep the test running, because await alone doesn't. +const err = new Error('rejected'); + +const p1 = import('../fixtures/es-modules/tla/await-export-promise.mjs') + .then(common.mustNotCall(), common.mustCall((e) => { + assert.strictEqual(e, err); + })); + +const p2 = import('../fixtures/es-modules/tla/await-export-promise.mjs') + .then(common.mustNotCall(), common.mustCall((e) => { + assert.strictEqual(e, err); + })); + +const p3 = import('../fixtures/es-modules/tla/await-export-promise.mjs') + .then(common.mustNotCall(), common.mustCall((e) => { + assert.strictEqual(e, err); + })); + +exportedReject(err); + +Promise.all([p1, p2, p3]).then(common.mustCall(() => { clearInterval(interval); })); diff --git a/test/es-module/test-require-module-retry-import-evaluating.js b/test/es-module/test-require-module-retry-import-evaluating.js new file mode 100644 index 00000000000000..df70432f1136e4 --- /dev/null +++ b/test/es-module/test-require-module-retry-import-evaluating.js @@ -0,0 +1,32 @@ +// This tests that after failing to require an ESM that contains TLA, +// retrying with import() still works, and produces consistent results. +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { exportedResolve } = require('../fixtures/es-modules/tla/export-promise.mjs'); + +assert.throws(() => { + require('../fixtures/es-modules/tla/await-export-promise.mjs'); +}, { + code: 'ERR_REQUIRE_ASYNC_MODULE' +}); + +const interval = setInterval(() => {}, 1000); // Keep the test running, because await alone doesn't. +const value = { hello: 'world' }; + +const p1 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => { + assert.strictEqual(ns.default, value); +}), common.mustNotCall()); + +const p2 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => { + assert.strictEqual(ns.default, value); +}), common.mustNotCall()); + +const p3 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => { + assert.strictEqual(ns.default, value); +}), common.mustNotCall()); + +exportedResolve(value); + +Promise.all([p1, p2, p3]).then(common.mustCall(() => { clearInterval(interval); })); diff --git a/test/es-module/test-require-module-tla-retry-import-2.js b/test/es-module/test-require-module-tla-retry-import-2.js new file mode 100644 index 00000000000000..aa9c344dd398d7 --- /dev/null +++ b/test/es-module/test-require-module-tla-retry-import-2.js @@ -0,0 +1,26 @@ +// This tests that after loading a ESM with import() and then retrying +// with require(), it errors as expected, and produces consistent results. +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +let ns; +async function test() { + const newNs = await import('../fixtures/es-modules/tla/export-async.mjs'); + if (ns === undefined) { + ns = newNs; + } else { + // Check that re-evalaution is returning the same namespace. + assert.strictEqual(ns, newNs); + } + assert.strictEqual(ns.hello, 'world'); + + assert.throws(() => { + require('../fixtures/es-modules/tla/export-async.mjs'); + }, { + code: 'ERR_REQUIRE_ASYNC_MODULE' + }); +} + +// Run the test twice to check consistency after caching. +test().then(common.mustCall(test)).catch(common.mustNotCall()); diff --git a/test/es-module/test-require-module-tla-retry-import.js b/test/es-module/test-require-module-tla-retry-import.js new file mode 100644 index 00000000000000..70f918fa4f463f --- /dev/null +++ b/test/es-module/test-require-module-tla-retry-import.js @@ -0,0 +1,25 @@ +// This tests that after failing to require an ESM that contains TLA, +// retrying with import() still works, and produces consistent results. +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +let ns; +async function test() { + assert.throws(() => { + require('../fixtures/es-modules/tla/export-async.mjs'); + }, { + code: 'ERR_REQUIRE_ASYNC_MODULE' + }); + const newNs = await import('../fixtures/es-modules/tla/export-async.mjs'); + if (ns === undefined) { + ns = newNs; + } else { + // Check that re-evalaution is returning the same namespace. + assert.strictEqual(ns, newNs); + } + assert.strictEqual(ns.hello, 'world'); +} + +// Run the test twice to check consistency after caching. +test().then(common.mustCall(test)).catch(common.mustNotCall()); diff --git a/test/fixtures/es-modules/tla/await-export-promise.mjs b/test/fixtures/es-modules/tla/await-export-promise.mjs new file mode 100644 index 00000000000000..0129793e42b54a --- /dev/null +++ b/test/fixtures/es-modules/tla/await-export-promise.mjs @@ -0,0 +1,4 @@ +import promise from './export-promise.mjs'; +let result; +result = await promise; +export default result; diff --git a/test/fixtures/es-modules/tla/export-async.mjs b/test/fixtures/es-modules/tla/export-async.mjs new file mode 100644 index 00000000000000..b6de9a5a5f68e9 --- /dev/null +++ b/test/fixtures/es-modules/tla/export-async.mjs @@ -0,0 +1,2 @@ +let hello = await Promise.resolve('world'); +export { hello }; diff --git a/test/fixtures/es-modules/tla/export-promise.mjs b/test/fixtures/es-modules/tla/export-promise.mjs new file mode 100644 index 00000000000000..74864d232e3c35 --- /dev/null +++ b/test/fixtures/es-modules/tla/export-promise.mjs @@ -0,0 +1,8 @@ +let exportedResolve; +let exportedReject; +const promise = new Promise((resolve, reject) => { + exportedResolve = resolve; + exportedReject = reject; +}); +export default promise; +export { exportedResolve, exportedReject };