Reify module instance

[kriskowal]

In this change, I propose the reification of ModuleInstance per @guybedford and @lucacasonato’s presentation at TC39 plenary today, June 8th. The premise is that ejecting ModuleInstance would allow us defer the definition of Loader to user-code, that StaticModuleRecord and ModuleInstance (by any other name) should be sufficient to construct a loader.

I found that I was unable to eliminate Loader from the design for two reasons:

1. Dynamic import ultimately needs to defer to a Loader.
2. Hardened JavaScript isolation of shared intrinsics requires a JavaScript object to serve as a token representing the global environment in which each module should be evaluated. Direct eval in turn requires that global environment record to check whether the lexical name eval corresponds to the intrinsic eval for that environment.

[kriskowal]

Threading a user-defined import(fullSpecifier: string) or removing the specifier argument and providing an import(importSpecifier: string) bound to the implied fullSpecifier might obviate one use of the Loader.

For hardened JavaScript isolation, we would need a way to construct a reified global environment record and pass that here instead of Loader.

[kriskowal]

I think I’ve convinced myself that Loader could be defined in user code if we instead provided all of import reflection, ModuleInstance, and a Global environment constructor. It would not be a net increase or decrease in expressivity, but would be a decrease in usability.

The Loader constructor makes it convenient to pass entries from its static record and instance memos from host to guest loader.

We would be significantly more dependent on import reflection to obtain reified StaticModuleRecords (by any name).

We would also lose Moddable’s support for passing static module records from host to guest by name. They would have to be passed by StaticModuleRecord object as obtained by import reflection instead.

[kriskowal]

In short, I don’t support this design direction, but wanted to give it a run in good faith and will of course be receptive if more folks prefer module loading without a Loader.

[kriskowal]

These reflective properties go from “nice to have” to “essential” when the Loader is user code and can’t reach into internal slots.

[Jack-Works]

Please consider #38

[Jack-Works]

How can this new design handles dynamic import?

[kriskowal]

A dynamic import hook must be passed as a ModuleInstance constructor option, which will throw if the static module record has needsImport set and import is not provided.

[guybedford]

This design seems right to me - in that the HostImportModuleDynamically hook effectively needs to be exposed as a function for each individual module instance in this way (along with the globals). Nice to see we agree on how it fleshes out!

Also agreed, the repeating of this information for all modules in the same compartment is clearly simplified by having a singular loader that can share these parameters between modules, and even in turn removes the need for the dynamic import hook even being necessary as well - and these are strong arguments for a loader over an unmanaged instance model.

[kriskowal]

I’m arriving at the harmonious conclusion that the union of Loader and ModuleInstance is necessary for this specification regardless of which ones get revealed to the end user. Existence of %ModuleInstance% is implied by Loader, and Loader is implied by %Realm%, so we may as well have both in the sketch and litigate later about what we ought to reveal to user code.

[ljharb]

Why would you want to be able to get different results between dynamically and statically importing the same specifier? (modulo thenable modules)

[guybedford]

You certainly wouldn't, which is also another possible argument for loader-level over instance-level management.

[kriskowal]

@Cardiy is also a proponent of exposing ModuleInstance and not exposing a Loader. A difference he proposes is that ModuleInstance would use an importHook(importSpecifier, moduleInstance) => Promise<ModuleInstance> hook for both static and dynamic import. There would need to be a link() method with no arguments that drives the importHook for static imports before initialization.

Caridy believes it would be sufficient for the ModuleInstance implementation to enforce local consistency of static and dynamic import by memoizing importHook locally. I also find that sufficient, but because it would be a more significant memory burden, and because the importHook would presumably want to enforce stronger consistency invariants among modules, I would be open to deferring the responsibility of maintaining the consistency invariant to the user-provided hook.

For the purposes of isolating guest code, the ModuleInstance would likewise need to accept an optional alternate global environment and I have not reckoned out whether that would be sufficient to implement a compartmental loader in user code.

Please let us know what you think. I will probably sketch that alternative view for purposes of discussion. I’m attracted to the idea of not having to convince TC39 of the exact shape of module descriptors, if that’s a possibility, though the Compartment API is still quite convenient.

[Jamesernator]

This method seems kind've strange given that it expects a specifier but the ModuleInstance itself has no way to reflect on what specifiers are requested by the record given to the constructor. Or even what the current link status of a given ModuleInstance is.

It would be helpful to expose what specifiers still need binding, so that we more easily write a linker:

async function link(moduleInstance: ModuleInstance) {
    // Need to know what specifiers still need linking so we can
    // recurse and link all the child modules as well
    for (const specifier of moduleInstance.unlinkedSpecifiers) {
        const module = await resolveModuleSomehow(specifier, moduleInstance);
        moduleInstance.link(specifier, module);
        // Link child modules
        await link(module);
    }
}

[kriskowal]

The ModuleInstance captures the underlying StaticModuleRecord, which in turn has a bindings array on which the Loader implementation (either user code or one built into the language), can derive a list of all the necessary linkage and track which one’s it has accounted for. Of course, the ModuleInstance will have to track all of this as well internally and maintain the invariant that a module specifier is only linked once, and that all links are satisfied before it is initialized.

[Jamesernator]

The ModuleInstance captures the underlying StaticModuleRecord, which in turn has a bindings array on which the Loader implementation (either user code or one built into the language), can derive a list of all the necessary linkage and track which one’s it has accounted for.

It looks like the latest changes make ModuleInstance a hidden spec object, so my initial comment is a moot point now.

[kriskowal]

Yes, I think we’re loosely converged on hiding %ModuleInstance% at this moment, but I’ve concluded it’s useful to keep and continue to develop the concept as a spec fiction, with the option of revealing it if it receives a lot of support.

[kriskowal]

I’ve pivoted the proposed change to introduce a %ModuleInstance% since the concept will be useful for spec text regardless of whether it’s reified as a JavaScript object or remains a spec fiction. I’ve also put Loader back, since there’s no version of this proposal where Loader will not have to exist at least as a spec fiction describing the loader behavior of a %Realm%, such that we can choose to hide %Loader% later. This puts us in a position where we agree that all these objects must be represented for their respective behavior and we can converge later on which ones we reveal.

[kriskowal]

I’ve captured the feedback from this conversation, I hope fairly and sufficiently tentatively.

[kriskowal]

I’ve rebased this on master so it can land ahead of #48, which I’ve demoted to draft.

[kriskowal]

I intend to reprise a pull request in this direction, per feedback from Caridy and positive responses from the Module Harmony call, but wish to leave this PR as-is to archive the state of our conversation.