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

When a ESM module cannot be loaded by require due to the presence of TLA, its module status would be stopped at kInstantiated. In this case, when it's imported again, we should allow it to be evaluated asynchronously, as it's also a common pattern for users to retry with dynamic import when require fails. PR-URL: #55502 Backport-PR-URL: #55217Fixes: #55500 Refs: #52697 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>

Author: joyeecheung

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{
 }
 
 asyncrun(){
+// This path is hit by a require'd module that is imported again.
 conststatus=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){
+awaitthis.evaluationPromise;
+}
+return{__proto__: null,module: this.module};
+}elseif(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.
+consttimeout=-1;
+constbreakOnSigint=false;
+this.evaluationPromise=this.module.evaluate(timeout,breakOnSigint);
+awaitthis.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';
+constcommon=require('../common');
+constassert=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'
+});
+
+constinterval=setInterval(()=>{},1000);// Keep the test running, because await alone doesn't.
+consterr=newError('rejected');
+
+constp1=import('../fixtures/es-modules/tla/await-export-promise.mjs')
+.then(common.mustNotCall(),common.mustCall((e)=>{
+assert.strictEqual(e,err);
+}));
+
+constp2=import('../fixtures/es-modules/tla/await-export-promise.mjs')
+.then(common.mustNotCall(),common.mustCall((e)=>{
+assert.strictEqual(e,err);
+}));
+
+constp3=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';
+constcommon=require('../common');
+constassert=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'
+});
+
+constinterval=setInterval(()=>{},1000);// Keep the test running, because await alone doesn't.
+constvalue={hello: 'world'};
+
+constp1=import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns)=>{
+assert.strictEqual(ns.default,value);
+}),common.mustNotCall());
+
+constp2=import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns)=>{
+assert.strictEqual(ns.default,value);
+}),common.mustNotCall());
+
+constp3=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';
+constcommon=require('../common');
+constassert=require('assert');
+
+letns;
+asyncfunctiontest(){
+constnewNs=awaitimport('../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';
+constcommon=require('../common');
+constassert=require('assert');
+
+letns;
+asyncfunctiontest(){
+assert.throws(()=>{
+require('../fixtures/es-modules/tla/export-async.mjs');
+},{
+code: 'ERR_REQUIRE_ASYNC_MODULE'
+});
+constnewNs=awaitimport('../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 @@
+importpromisefrom'./export-promise.mjs';
+letresult;
+result=awaitpromise;
+exportdefaultresult;

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

@@ -0,0 +1,2 @@
+lethello=awaitPromise.resolve('world');
+export{hello};

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

@@ -0,0 +1,8 @@
+letexportedResolve;
+letexportedReject;
+constpromise=newPromise((resolve,reject)=>{
+exportedResolve=resolve;
+exportedReject=reject;
+});
+exportdefaultpromise;
+export{exportedResolve,exportedReject};