Skip to content

Commit 21c9e24

Browse files
MayaLekovaMayaLekova
committed
module: implement deferred import for dynamic modules
This PR adds support for deferring the import of modules in additional cases, such as importing a dynamic module. It also adds a test for this case. Refs: https://github.com/tc39/proposal-defer-import-eval Signed-off-by: Maya Lekova <maya@igalia.com>
1 parent 9df97a7 commit 21c9e24

12 files changed

Lines changed: 133 additions & 38 deletions

‎lib/internal/bootstrap/realm.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,10 @@ const {
183183
setInternalLoaders,
184184
} = internalBinding('builtins');
185185

186-
const { ModuleWrap } = internalBinding('module_wrap');
186+
const {
187+
ModuleWrap,
188+
kEvaluationPhase,
189+
} = internalBinding('module_wrap');
187190
ObjectSetPrototypeOf(ModuleWrap.prototype, null);
188191

189192
const getOwn = (target, property, receiver) => {
@@ -402,7 +405,7 @@ class BuiltinModule {
402405
});
403406
// Ensure immediate sync execution to capture exports now
404407
this.module.instantiate();
405-
this.module.evaluate(-1, false);
408+
this.module.evaluate(-1, false, kEvaluationPhase);
406409
return this.module;
407410
}
408411

‎lib/internal/modules/esm/loader.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ class ModuleLoader {
692692
return module.getModuleSourceObject();
693693
}
694694
const { module } = await moduleJob.run(isEntryPoint);
695-
return module.getNamespace();
695+
return module.getNamespace(phase);
696696
}, {
697697
__proto__: null,
698698
parentURL,

‎lib/internal/modules/esm/module_job.js‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ class ModuleJobBase {
329329
debug(`ModuleJobBase.syncLink() ${this.url} -> ${request.specifier}`, job);
330330
assert(!isPromise(job));
331331
assert(job.module instanceof ModuleWrap);
332-
if (this.shouldRunModule(request.phase)) {
332+
if (this.shouldEvaluateModule(request.phase)) {
333333
ArrayPrototypePush(evaluationDepJobs, job);
334334
}
335335
modules[idx] = job.module;
@@ -383,7 +383,7 @@ class ModuleJobBase {
383383
shouldLinkModule(phase) {
384384
return phase >= kDeferPhase;
385385
}
386-
shouldRunModule(phase) {
386+
shouldEvaluateModule(phase) {
387387
return phase === kEvaluationPhase;
388388
}
389389
}
@@ -466,7 +466,7 @@ class ModuleJob extends ModuleJobBase {
466466
const dependencyJobPromise = this.loader.getOrCreateModuleJob(this.url, request, requestType);
467467
const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
468468
debug(`ModuleJob.asyncLink() ${this.url} -> ${request.specifier}`, job);
469-
if (this.shouldRunModule(request.phase)) {
469+
if (this.shouldEvaluateModule(request.phase)) {
470470
ArrayPrototypePush(evaluationDepJobs, job);
471471
}
472472
return job.modulePromise;
@@ -521,11 +521,11 @@ class ModuleJob extends ModuleJobBase {
521521
}
522522

523523
runSync(parent) {
524-
assert(this.shouldRunModule(this.phase));
524+
assert(this.shouldEvaluateModule(this.phase));
525525
assert(this.module instanceof ModuleWrap);
526526
let status = this.module.getStatus();
527527

528-
debug('ModuleJob.runSync()', status, this.module);
528+
debug(`ModuleJob.runSync(); status: ${status}`, this.module);
529529
if (status === kUninstantiated) {
530530
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that
531531
// the async graph error supersedes instantiation (mismatch export) errors in the graph.
@@ -541,7 +541,8 @@ class ModuleJob extends ModuleJobBase {
541541
this.throwIfAsyncGraph(parent);
542542
if (status === kInstantiated) {
543543
setHasStartedUserESMExecution();
544-
const namespace = this.module.evaluateSync();
544+
const namespace = this.phase === kEvaluationPhase ?
545+
this.module.evaluateSync() : this.module.evaluateSync(this.phase);
545546
return { __proto__: null, module: this.module, namespace };
546547
}
547548
throw this.module.getError();
@@ -561,17 +562,17 @@ class ModuleJob extends ModuleJobBase {
561562
}
562563

563564
async run(isEntryPoint = false) {
564-
debug('ModuleJob.run()', this.module);
565-
assert(this.shouldRunModule(this.phase));
565+
debug('ModuleJob.run(); phase: ', this.phase, this.module);
566566
await this.#instantiate();
567567
if (isEntryPoint) {
568568
globalThis[entry_point_module_private_symbol] = this.module;
569569
}
570+
570571
const timeout = -1;
571572
const breakOnSigint = false;
572573
setHasStartedUserESMExecution();
573574
try {
574-
await this.module.evaluate(timeout, breakOnSigint);
575+
await this.module.evaluate(timeout, breakOnSigint, this.phase);
575576
} catch (e) {
576577
explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait);
577578
throw e;
@@ -641,9 +642,9 @@ class ModuleJobSync extends ModuleJobBase {
641642
}
642643

643644
async run(isEntryPoint = false) {
644-
assert(this.shouldRunModule(this.phase));
645+
// This path is hit by a require'd module that is imported again.
645646
const status = this.module.getStatus();
646-
debug('ModuleJobSync.run()', status, this.module);
647+
debug(`ModuleJobSync.run() at phase ${this.phase}; status: ${status}`, this.module);
647648
// If the module was previously required and errored, reject from import() again.
648649
if (status === kErrored) {
649650
throw this.module.getError();
@@ -674,17 +675,18 @@ class ModuleJobSync extends ModuleJobBase {
674675
const breakOnSigint = false;
675676
if (this.inspectBrk) {
676677
const { callAndPauseOnStart } = internalBinding('inspector');
677-
this.evaluationPromise = callAndPauseOnStart(this.module.evaluate, this.module, timeout, breakOnSigint);
678+
this.evaluationPromise = callAndPauseOnStart(
679+
this.module.evaluate, this.module, timeout, breakOnSigint, this.phase);
678680
} else {
679-
this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint);
681+
this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint, this.phase);
680682
}
681683
await this.evaluationPromise;
682684
this.evaluationPromise = undefined;
683685
return { __proto__: null, module: this.module };
684686
}
685687
// No top-level `await`: evaluate synchronously
686688
try {
687-
this.#evaluateSync();
689+
this.#evaluateSync(this.phase);
688690
} catch (evaluateError) {
689691
explainCommonJSGlobalLikeNotDefinedError(evaluateError, this.module.url, this.module.hasTopLevelAwait);
690692
throw evaluateError;
@@ -694,17 +696,23 @@ class ModuleJobSync extends ModuleJobBase {
694696

695697
// Evaluate this module synchronously, pausing at the first line of user land JS
696698
// that runs if --inspect-brk is active - that may well be inside a dependency.
697-
#evaluateSync() {
699+
#evaluateSync(phase) {
698700
if (this.inspectBrk) {
699701
const { callAndPauseOnStart } = internalBinding('inspector');
700-
return callAndPauseOnStart(this.module.evaluateSync, this.module);
702+
if (phase === kEvaluationPhase) {
703+
return callAndPauseOnStart(this.module.evaluateSync, this.module);
704+
}
705+
return callAndPauseOnStart(this.module.evaluateSync, this.module, phase);
706+
}
707+
if (phase !== kEvaluationPhase) {
708+
return this.module.evaluateSync(phase);
701709
}
702710
return this.module.evaluateSync();
703711
}
704712

705713
runSync(parent) {
706714
debug('ModuleJobSync.runSync()', this.module);
707-
assert(this.shouldRunModule(this.phase));
715+
assert(this.shouldEvaluateModule(this.phase));
708716
// TODO(joyeecheung): Reject graphs with top-level await _before_ instantiation, so that the
709717
// async graph error supersedes instantiation (mismatch export) errors in the graph.
710718
this.instantiateSync();
@@ -714,7 +722,7 @@ class ModuleJobSync extends ModuleJobBase {
714722
this.throwIfAsyncGraph(parent);
715723
setHasStartedUserESMExecution();
716724
try {
717-
const namespace = this.#evaluateSync();
725+
const namespace = this.#evaluateSync(this.phase);
718726
return { __proto__: null, module: this.module, namespace };
719727
} catch (e) {
720728
explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait);

‎lib/internal/modules/esm/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525

2626
const {
2727
ModuleWrap,
28+
kEvaluationPhase,
2829
setImportModuleDynamicallyCallback,
2930
setInitializeImportMetaObjectCallback,
3031
} = internalBinding('module_wrap');
@@ -383,7 +384,7 @@ function compileSourceTextModule(url, source, type, context = kEmptyObject) {
383384
const modules = requests.map(({ specifier }) => getBuiltinModuleWrapForEmbedder(specifier));
384385
wrap.link(modules);
385386
wrap.instantiate();
386-
wrap.evaluate(-1, false);
387+
wrap.evaluate(-1, false, kEvaluationPhase);
387388
}
388389
return wrap;
389390
}

‎lib/internal/vm/module.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ class Module {
230230
'must be one of linked, evaluated, or errored',
231231
);
232232
}
233-
return this[kWrap].evaluate(timeout, breakOnSigint);
233+
return this[kWrap].evaluate(timeout, breakOnSigint, kEvaluationPhase);
234234
} catch (e) {
235235
return PromiseReject(e);
236236
}

‎src/module_wrap.cc‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,17 @@ ModulePhase to_phase_constant(ModuleImportPhase phase) {
564564
}
565565
}
566566

567+
ModuleImportPhase to_module_import_phase(ModulePhase phase) {
568+
switch (phase) {
569+
case kEvaluationPhase:
570+
return ModuleImportPhase::kEvaluation;
571+
case kDeferPhase:
572+
return ModuleImportPhase::kDefer;
573+
default:
574+
UNREACHABLE();
575+
}
576+
}
577+
567578
static Local<Object> createImportAttributesContainer(
568579
Realm* realm,
569580
Isolate* isolate,
@@ -774,8 +785,8 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
774785
if (contextify_context != nullptr)
775786
microtask_queue = contextify_context->microtask_queue();
776787

777-
// module.evaluate(timeout, breakOnSigint)
778-
CHECK_EQ(args.Length(), 2);
788+
// module.evaluate(timeout, breakOnSigint, phase)
789+
CHECK_EQ(args.Length(), 3);
779790

780791
CHECK(args[0]->IsNumber());
781792
int64_t timeout;
@@ -786,6 +797,9 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
786797
CHECK(args[1]->IsBoolean());
787798
bool break_on_sigint = args[1]->IsTrue();
788799

800+
CHECK(args[2]->IsInt32());
801+
ModulePhase phase = static_cast<ModulePhase>(args[2].As<Int32>()->Value());
802+
789803
ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env());
790804
TryCatchScope try_catch(realm->env());
791805

@@ -800,7 +814,13 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
800814
isolate, &received_signal)
801815
: std::nullopt;
802816

803-
result = module->Evaluate(context);
817+
CHECK_NE(phase, kSourcePhase);
818+
if (phase == kDeferPhase) {
819+
result = module->EvaluateForImportDefer(context);
820+
} else {
821+
CHECK_EQ(phase, kEvaluationPhase);
822+
result = module->Evaluate(context);
823+
}
804824

805825
Local<Value> res;
806826
if (result.ToLocal(&res) && microtask_queue) {
@@ -921,10 +941,26 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
921941
Local<Module> module = obj->module_.Get(isolate);
922942
Environment* env = realm->env();
923943

944+
// module.evaluateSync([phase])
945+
CHECK_LE(args.Length(), 1);
946+
ModulePhase phase = kEvaluationPhase;
947+
if (args.Length() > 0) {
948+
CHECK(args[0]->IsInt32());
949+
phase = static_cast<ModulePhase>(args[0].As<Int32>()->Value());
950+
}
951+
924952
Local<Value> result;
925953
{
954+
MaybeLocal<Value> maybe_result;
926955
TryCatchScope try_catch(env);
927-
if (!module->Evaluate(context).ToLocal(&result)) {
956+
if (phase == kDeferPhase) {
957+
maybe_result = module->EvaluateForImportDefer(context);
958+
} else {
959+
CHECK_EQ(phase, kEvaluationPhase);
960+
maybe_result = module->Evaluate(context);
961+
}
962+
963+
if (!maybe_result.ToLocal(&result)) {
928964
if (try_catch.HasCaught()) {
929965
if (!try_catch.HasTerminated()) {
930966
try_catch.ReThrow();
@@ -954,12 +990,19 @@ void ModuleWrap::GetNamespace(const FunctionCallbackInfo<Value>& args) {
954990
ModuleWrap* obj;
955991
ASSIGN_OR_RETURN_UNWRAP(&obj, args.This());
956992

993+
ModulePhase phase = kEvaluationPhase;
994+
if (args.Length() > 0) {
995+
CHECK(args[0]->IsInt32());
996+
phase = static_cast<ModulePhase>(args[0].As<Int32>()->Value());
997+
}
998+
957999
Local<Module> module = obj->module_.Get(isolate);
9581000
if (module->GetStatus() < Module::kInstantiated) {
9591001
return THROW_ERR_MODULE_NOT_INSTANTIATED(realm->env());
9601002
}
9611003

962-
Local<Value> result = module->GetModuleNamespace();
1004+
Local<Value> result =
1005+
module->GetModuleNamespace(to_module_import_phase(phase));
9631006
args.GetReturnValue().Set(result);
9641007
}
9651008

‎test/es-module/test-defer-dynamic-import.mjs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Flags: --js-defer-import-eval
2+
3+
// Tests that defer import of a dynamically imported module
4+
// defers the execution of the module until any of its properties
5+
// is accessed for the first time.
6+
7+
import * as common from '../common/index.mjs';
8+
import * as assert from 'assert';
9+
10+
globalThis.eval_list = [];
11+
12+
await import.defer('../fixtures/es-modules/module-deferred-eval.mjs').then(
13+
(module) => {
14+
// The module evaluation should be deferred until the namespace (that is,
15+
// any of its exported properties) is accessed.
16+
assert.deepStrictEqual(globalThis.eval_list, [],
17+
new SyntaxError('imported module should have been deferred until first property access'));
18+
19+
// Accessing the namespace should trigger the evaluation of the module.
20+
assert.strictEqual(module.foo, 42);
21+
assert.deepStrictEqual(globalThis.eval_list, ['defer-1'],
22+
new Error('imported module should have been executed only once'));
23+
24+
// Clean-up
25+
delete globalThis.eval_list;
26+
}).then(common.mustCall());

‎test/es-module/test-defer-import-with-module-tree.mjs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Flags: --js-defer-import-eval
22

33
import '../common/index.mjs';
4+
import * as assert from 'assert';
5+
6+
globalThis.eval_list = [];
47

58
import defer * as deferred from '../fixtures/es-modules/module-with-module-tree.mjs';
69

7-
console.log(deferred.bar);
10+
assert.strictEqual(deferred.bar, 64);
811

912
delete globalThis.eval_list;

‎test/es-module/test-defer-import-with-regular-import.mjs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,19 @@
77
import '../common/index.mjs';
88
import * as assert from 'assert';
99

10-
globalThis.eval_list ||= [];
10+
globalThis.eval_list = [];
1111

1212
import * as non_deferred from '../fixtures/es-modules/module-with-module-tree.mjs';
1313
import defer * as deferred from '../fixtures/es-modules/module-deferred-eval.mjs';
1414

1515
// Check that the module has been evaluated exactly once.
16-
assert.strictEqual(globalThis.eval_list.length, 1);
1716
assert.strictEqual(deferred.foo, 42);
17+
assert.strictEqual(globalThis.eval_list.length, 1);
1818
assert.partialDeepStrictEqual(['defer-1'], globalThis.eval_list);
1919

2020
// Skip linter warning
2121
assert.strictEqual(non_deferred.bar, 64);
22+
assert.strictEqual(globalThis.eval_list.length, 1);
2223

2324
// Clean-up
2425
delete globalThis.eval_list;

‎test/fixtures/es-modules/module-deferred-eval.mjs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
if (!globalThis.eval_list) {
2-
globalThis.eval_list = [];
3-
}
1+
// This fixture is imported as a module in a few tests for `import defer`
2+
// to ensure that a module imported with `import defer` is only executed once
3+
// and its evaluation is deffered.
4+
import * as assert from 'assert';
45

56
export const foo = 42;
7+
8+
// The `eval_list` is initialised by the importing module,
9+
// so by the time the fixture is executed, `eval_list` should
10+
// already be initialised.
11+
assert.deepEqual(globalThis.eval_list, []);
12+
613
globalThis.eval_list.push('defer-1');

0 commit comments

Comments
 (0)