Stage 3 and 4 roadmap

[nicolo-ribaudo]

Pending from Stage 2.7

Stage 2.7 was conditional on the editors finishing reviewing the spec text. As far as I'm concerned the proposal is now in its final shape, so this is a good time to review it :)

• Editor reiviews
  • @bakkot
  • @michaelficarra
  • @syg

Stage 3

• test262 (import defer testing plan test262#4215)
  • Add partial tests for import defer Add partial tests for import defer test262#4278
  • Add test for @@toStringTag on deferred module namespaces Add test for @@toStringTag on deferred module namespaces test262#4360
  • Update import defer tests for evaluation triggers Update import defer tests for evaluation triggers test262#4341
  • Add syntax tests for import.defer Add syntax tests for import.defer test262#4374
  • Add evaluation tests for import.defer with sync/async modules Add evaluation tests for import.defer with sync/async modules test262#4375

Stage 4

• ECMA-262 PR
• Two implementations
  • JSC (parsing support: [JSC] Add parsing for deferred import proposal WebKit/WebKit#36664, semantics: [JSC] Add core semantics of import defer WebKit/WebKit#40453)
  • SM (tracking bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1898544)
  • V8 (tracking bug: https://issues.chromium.org/issues/398218423)

[ljharb]

I believe the actual reviewers need to approve/reaffirm as well, not just the editors.

[guybedford]

Just did another review of the spec text and can confirm everything continues to look good to me.

One minor change I'd suggest using 16.2.1.10 GetModuleNamespace ( module, deferred ) with deferred as a boolean instead of the GetModuleNamespace( module, phase ) taking a phase argument, since we don't anticipate defining other variations of module namespaces at other phases.

[dminor]

Looks good to me as well.

[michaelficarra]

• In ModuleNamespaceCreate, step 11, why are we hand-waving creating a data property instead of going through [[DefineOwnProperty]]?
• The parameters of IsSymbolLikeNamespaceKey and GetModuleExportsList are typed as "Module Namespace Exotic Object" but should be typed as "module namespace exotic object" to properly link to the <dfn>.
• IsSymbolLikeNamespaceKey: "corresponds to a potential ordinary property" -> "corresponds to an ordinary property"
• GetModuleExportsList: "whose elements are the String values of the names of" -> "whose elements are the names of"
• ReadyForSyncExecution and GatherAsynchronousTransitiveDependencies: "If seen is not provided, let seen be a new empty List." -> "If seen is not present, set seen to a new empty List."
• Static Semantics: ModuleRequests: I think Shu may have said this elsewhere, but let's not do the manual iteration here. Just use "contains" and "such that".
• Follow the definite article rule for SDO invocations, which apparently applies to SV. But also applies to others like ImportEntriesForModule.
• Module Record Fields table: markup issues for true/false
• Additional Fields of Cyclic Module Records table: "along with their maximum specified import phase". I don't know what this is supposed to mean. Is there an ordering given somewhere for the spec enums used for import phases? I don't think so. Note that this is different than the enum values used for [[Status]] which have been given an order. I would change this to just talk about ModuleRequest Records.
• Evaluate: "set to evaluating or a status earlier than linked". I would just enumerate the statuses that they are or are not allowed to be. I don't want the reader to have to refer to the order specified in the Additional Fields of Cyclic Module Records table.
• EvaluateSync: "and all its asynchronous transitive dependencies have already been evaluated". Is it clear what this means? Can we be more precise?
• EvaluateSync: You can just return ~unused~. You don't need to use NormalCompletion when the AO type says it returns a completion record. Similarly, "Return ThrowCompletion" can just be "throw". See https://tc39.es/ecma262/#sec-throw-an-exception.
• GatherAsynchronousTransitiveDependencies nit: pick one of "asynchronous" or "async" but not both. I prefer the former.
• GetModuleNamespace: ", otherwise" -> ". Otherwise"
• GetModuleNamespace: How can this AO return ~empty~? I don't see how it could and the version in ECMA-262 is not marked as returning ~empty~.

Still LGTM, this is all minor.

• Related ecma262 editorial change: we should <dfn> "Module Namespace Object" and remove the <emu-xref> in #table-module-record-fields.

[michaelficarra]

@guybedford

One minor change I'd suggest using 16.2.1.10 GetModuleNamespace ( module, deferred ) with deferred as a boolean instead of the GetModuleNamespace( module, phase ) taking a phase argument, since we don't anticipate defining other variations of module namespaces at other phases.

I think I'd disagree with this. I think it's better to take a two-state spec enum phase than a boolean.

[nicolo-ribaudo]

@michaelficarra #61

I also have some comments:

• In ModuleNamespaceCreate, step 11, why are we hand-waving creating a data property instead of going through [[DefineOwnProperty]]?

The object is exhotic, and going through [[DefineOwnProperty]] will:

• check that the key is a symbol, and then run OrdinaryDefineOwnProperty.
• OrdinaryDefineOwnProperty will the check if the object is extensible
• [[IsExtensible]] then returns false (https://tc39.es/ecma262/#sec-module-namespace-exotic-objects-isextensible), so ValidateAndApplyPropertyDescriptor will not actually create the property.

The idea is that this object is effectively born with that Symbol.toStringTag, from creation. Note that, for the same reason, the current ecma262 definition of namespace objects is similarly hand-wavy ("Create own properties of M corresponding to the definitions in 28.3.").

• IsSymbolLikeNamespaceKey: "corresponds to a potential ordinary property" -> "corresponds to an ordinary property"

The property might either be ordinary, or not be there at all: that's the reason why I wrote "potential". I see though that my word is confusing, since a "potential ordinary property" just means that either it's an ordinary property or it's not. I also find your wording slightly confusing, since it reads like it's actually checking that that object has that property and that it's ordinary.

I'm going to go ahead and change it to "It determines if a property of ns with key P would be an ordinary property, rather than being mapped to one of the module exports.", but let me know if you prefer your original suggestion.

• Static Semantics: ModuleRequests: I think Shu may have said this elsewhere, but let's not do the manual iteration here. Just use "contains" and "such that".

Yeah Shu pointed this out in the import attributes pull request. I'm going to change the diff in that whole ModuleRequest Record section to be on top of the import attributes PR rather than on the current ecma262, so that iteration is just what's in the PR.

• Additional Fields of Cyclic Module Records table: "along with their maximum specified import phase". I don't know what this is supposed to mean. Is there an ordering given somewhere for the spec enums used for import phases? I don't think so. Note that this is different than the enum values used for [[Status]] which have been given an order. I would change this to just talk about ModuleRequest Records.

Mh yeah that's just wrong, it's some outdated text based on an outdated version of the proposal. What I tried to convey by maximum is that if you have both import defer and import for the same module, that list will mark the phase as "evaluation", because the combination of the two imports causes the module to be evaluated. Now the list contains both import declarations.

I'll just copy the description used in the import attributes PR: "A List of the ModuleRequest Records associated with the imports in this module.".

(And actually, I'll just remove all those tables in Cyclic Module Record since they have no actual diff compared to ECMA-262 + import attributes)

• EvaluateSync: "and all its asynchronous transitive dependencies have already been evaluated". Is it clear what this means? Can we be more precise?

Changing to "For each Cyclic Module Record m that it a transitive dependency of module, either m.[[HasTLA]] is false or m.[[Status]] is evaluated."

• GetModuleNamespace: How can this AO return ~empty~? I don't see how it could and the version in ECMA-262 is not marked as returning ~empty~.

Yep, it cannot.

[nicolo-ribaudo]

@syg @bakkot If it helps, given that you just reviewed the import attributes PR, #62 is this proposal rebased on top of it (it mostly reduces the diff, + some changes to the grammar to account defer and with together).

[guybedford]

I think I'd disagree with this. I think it's better to take a two-state spec enum phase than a boolean.

The problem is with three states, source makes no sense for this function so would need to be asserted out.

[nicolo-ribaudo]

The type of this specific function can still be just ~evaluation~ or ~defer~ even once we introduce the source phase.

[linusg]

Implemented most of this today, issues I found along the way in addition to #65:

• ParseModule() and CreateDefaultExportSyntheticModule() are missing [[DeferredNamespace]] : empty in the module record
• ReadyForSyncExecution() is typed as taking a Cyclic Module Record but calls itself recursively with an arbitrary Module Record (result of GetImportedModule()) - I think this needs a type check before recursing
• ✅ fixed in 437182d EvaluateImportCall() step 13 creates a ModuleRequest Record without the mandatory [[Phase]] field
  • Since this is being passed to HostLoadImportedModule() maybe the DynamicImportState Record change won't be needed?