Suggestion: only one valid `type` per resolved URL

[GeoffreyBooth]

I’m working on implementing this for Node.js. I have some suggestions for this proposal:

1. For JavaScript module imports, the lack of an assertion type implies assert { type: 'javascript' } to match the HTML spec. Therefore import './file.js' is the interchangeable equivalent of import './file.js' assert { type: 'javascript' }. If code contains both, the runtime will behave as if each import statement had been written with the full explicit assertion.

2. All non-JavaScript assertion types should be required. So if/when WebAssembly is supported, assert { type: 'webassembly' } would be required to import it, regardless of whether or not it’s ultimately decided that WebAssembly is as privileged as JavaScript; and likewise for all other future types.

In other words, within a module graph a resolved URL can have only one valid type. There cannot be two successfully resolved modules for the same URL where the modules have different types, even if one of the types is the implicit javascript.

This came out of discussing the expected behavior of code like this:

const jsonUrl = 'data:application/json,""'

const results = await Promise.allSettled([
  import(jsonUrl),
  import(jsonUrl, { assert: { type: 'json' } }),
])

console.log(results.map(result => result.status))

In Chrome, this prints ["rejected", "fulfilled"]. If you flip the order of the import statements, it prints ["fulfilled", "rejected"]. This behavior makes sense to me, and it follows the HTML spec’s statement that “module type is also part of the module map key.”

However, this is in conflict with this proposal’s statement:

Assertions are not part of the module cache key. Implementations are required to return the same module, or an error, regardless of the assertions.

I think that type, at least, needs to be part of the module cache key. If it weren’t, in the example above one would get ["fulfilled", "fulfilled"] or ["rejected", "rejected"] depending on the ordering of the import statements or a race of which resolved first. This feels like a footgun at best and a bug at worst.

There is a way to have it all, however, where we can avoid race conditions yet still have only one module in the cache per resolved URL: permit only one module to successfully resolve per URL and type. This is what Chrome is already doing for JSON. We should codify this in the spec, so that all runtimes behave the same way.

All non-JavaScript module types, including WebAssembly, should need an explicit assertion type so that there would be only one possible successful module imported for a particular URL. Multiple imports of the same URL with different types would create a race condition, but as long as the race can have only one winner then the condition doesn’t present a problem. This makes the implementation much simpler, as the runtime doesn’t need to contemplate the possibility of multiple valid modules for the same URL.

[ljharb]

The entire progression of this feature assumed that environments would want to, and would, make some assertions optional. Without that possibility, the feature would never have achieved stage 2, let alone stage 3.

[devsnek]

For your first point, the specification lacks a global module cache, so the strongest requirement it can make is to imports of the same specifier in the same module, which is not a very useful requirement. Given that hosts can always apply their own more-stringent requirements on top of the spec (see html for an example), it isn't really needed anyway. This has previous discussion in this repo and in plenary (notes are available). I encourage you to use github's search to find this information.

For your second point, hosts already provide non-javascript modules for bare imports. The most notable would be Node.js, which uses synthetic modules for builtins. There are also various embedded platforms which do similar.

Also side note on caching, the specific spec text in question is this:

• Any subsequent call to HostResolveImportedModule after FinishDynamicImport has completed, given the arguments referencingScriptOrModule, and moduleRequest must complete normally.
• If the host environment takes the success path once for a given referencingScriptOrModule, moduleRequest pair, it must always do so for subsequent calls.

This requirement matches chrome's implementation.

[bmeck]

None of the examples @GeoffreyBooth gave above are cross module so the point about global cache is somewhat irrelevant.

This suggestion is to upstream those stricter host requirements onto the JS spec it seems so that it can be use across JS environments in a guaranteed manner.

This is also feedback from implementation and outside of theory crafting feedback we have seen prior to this in the repo so simply searching wouldn't be enough as this is a new avenue that is bringing the discussion back to the proposal.

For your second point, hosts already provide non-javascript modules for bare imports. The most notable would be Node.js, which uses synthetic modules for builtins. There are also various embedded platforms which do similar.

Yes, but the implied type of those modules could be something else and importing with type:javascript could fail. The subject at hand is if 2 different types (such as a "undefined" type w/o an implicit default like HTML provides [HTML provides ["javascript"](https://html.spec.whatwg.org/multipage/webappapis.html#fetch-an-import()-module-script-graph)]) can collide. For modules in Node we already have an implied type of builtin and not javascript.

This requirement matches chrome's implementation.

This is a suggestion to change the proposal, saying that an implementation matches the existing spec text in response to confusion likely means the spec text needs updating or there is an ambiguity that is newly seen. Either way this sounds like we should investigate either route.

[devsnek]

The title of the issue is "only one valid type per resolved URL" which seems like its asking for global consistency? Hopefully geoffrey can clarify.

All non-JavaScript assertion types should be required the implied type of those modules could be something else

?

[ljharb]

The spec is explicitly designed to allow browsers’ use case - type json is required - and node’s use case - type json is optional.

Thjngs that aren’t browsers shouldn’t be constraining themselves to browser requirements; there’s a reason the spec has an entire annex designated only for browsers.

[bmeck]

@devsnek even local cache is resolved to some module record.

@ljharb allow but does not mean it is practical.

Thjngs that aren’t browsers shouldn’t be constraining themselves to browser requirements; there’s a reason the spec has an entire annex designated only for browsers.

I find this argument tedious given nothing in this issue's suggestion was about wanting to ensure all browser constraints are applied to node. If this could be narrowed to the specific reason why the confusion/collision of the types is practical to implement and not a burden it would be good to hear. Remember that ESM was also designed to allow Synchronous execution but cannot feasibly do so in practice.

[guybedford]

Also side note on caching, the specific spec text in question is this:

Any subsequent call to HostResolveImportedModule after FinishDynamicImport has completed, given the arguments referencingScriptOrModule, and moduleRequest must complete normally. If the host environment takes the success path once for a given referencingScriptOrModule, moduleRequest pair, it must always do so for subsequent calls.

This requirement matches chrome's implementation.

@devsnek are we sure about this? import(jsonModule) failing after completing succesfully with the assertion seems like it might be in violation of the above text?

[guybedford]

@devsnek are we sure about this

Apologies, took the trouble to actually read the spec and of course it's all nicely consistent!