module: allow ESM that failed to be required to be re-imported

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() {

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); }));

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); }));

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());

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());

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;

test/fixtures/es-modules/tla/export-async.mjs

@@ -0,0 +1,2 @@
+let hello = await Promise.resolve('world');
+export { hello };

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 };