Layering: Add GetRequestedModules() to Module Record

[dandclark]

Add the GetRequestedModules() abstract method to Abstract Module Record, with a corresponding implementation for Source Text Module Record. Move Cyclic Module Record's [[RequestedModules]] down to Source Text Module Record, as it is now used directly only by STMR, and its presence on Cyclic MR causes problems for derived module types like HTML Module Record that don't necessarily have strings associated with all their child modules.

The purpose of GetRequestedModules() is to abstract out from InnerModuleInstantiation/InnerModuleEvaluation the details of how a Module Record type stores its requested modules, since these are different for HTML Module Records as implemented here.

It is not time yet to merge this in given that there are still open questions about the HTML Modules design (mostly tracked over in the w3c/webcomponents repo). The intent here is to facilitate discussion and tease out issues that can be better found when making changes against the actual spec, with the intention of eventually merging in the updated PR once consensus has been reached on all open issues.

I've placed the built result of this change here: https://dandclark.github.io/built-specs/ecma262/get-requested-modules.html

[littledan]

This change seems good to me. It'll cause a tiny bit of duplication for WebAssembly modules, but that's fine.

A nit: Maybe [[RequiredModules]] should move down to SourceTextModuleRecord if its usages are exclusively there.

Thanks for following up with these appropriate layering changes; I am excited to see HTML modules moving forward.

[littledan]

The specification previously had ? rather than ! from some paths, so I think it would be best to leave it that way here. Then, the two callsites can invoke it with either of those, based on whether they anticipate errors.

[dandclark]

Thanks! I like the idea of moving [[RequiredModules]] down to STMR...I'll look into this.

[dandclark]

I pushed an update that:

• Switched '?' to '!' in GetRequestedModules and removed the note stating that the HostResolveImportedModules call must have succeeded.
• Pushed [[RequestedModules]] down to Source Text Module Record.

Thanks for the comments!

[littledan]

Nit: I see this marked as "Normative" in the PR title. However, I don't know what normative changes are in this PR. To me, it looks more like a "Layering" (or "Editorial") PR, which should be possible to land without tests or prior review in plenary (unless it seems controversial or worth noting in committee).

[ljharb]

Layering PRs are often normative; and we don’t have a different commit category for them. In particular, almost any change to Modules is effectively normative due to the partial nature in which 262 specifies them.

[littledan]

Just for context, we have used the "Layering:" prefix on commits before, e.g., #242 , even if I failed to list this when writing our current CONTRIBUTING.md. I still don't understand how this patch is normative, in a way that would make other layering patches not normative (since embedders can hook into any of the layering changes). I don't expect any JavaScript programs to change their behavior due to this change.

[ljharb]

That's a fair point, and maybe that makes this one editorial; either way, I'd expect anything that impacts Modules, especially CMRs and STMRs, to require committee consensus (otherwise the "dynamic modules" agenda item wouldn't have required it either).

[dandclark]

Yeah, when creating the PR I thought it didn't quite fit into either 'Normative' or 'Editorial' but didn't know there was precedent for using 'Layering' as a category. :) I've updated the title to 'Layering'.

In any case I agree that a module chage like this likely requires committee consensus.

[littledan]

I don't see any issues with this PR. Good work! However, maybe we should wait to land it until there's a bit more clarity on what the semantics of HTML modules will be.

[zenparsing]

I'm curious if the desired indirection cannot instead be applied in HostResolveImportedModule.

The [[RequestedModules]] list can be any arbitrary strings: their semantics depends upon the module record that contains them. A cyclic module record m could have auto-generated keys in [[RequestedModules]], say #1 and #2. HostResolveImportedModule(m, '#1') would then resolve to the desired dependency.