Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce interoperable behavior for unsupported 'type' assertion values #111

Closed

Conversation

dandclark
Copy link
Collaborator

@dandclark dandclark commented Mar 12, 2021

In #27 there was discussion about what to do with unrecognized types. If we want to enforce that unsupported 'type' assertion values always trigger a failure, this is one way to do it.

This change enforces that unsupported values of the 'type' assertion will trigger a failure. This is done with the introduction of the host-defined operation HostGetSupportedExtraModuleScriptTypes. Hosts will use this operation to indicate which module types (other than JavaScript) they support. If an import statement includes a type assertion that the host does not support, it will result in a syntax error (or TypeError with dynamic import()).

The result is unsupported types will be treated the same on all hosts, rather than some hosts ignoring them and some hosts triggering failure.

This change is written to avoid the scenario where a host could decide whether or not either of the following are supported:

import "./foo.js" assert { type: "js" };
import "./foo.js" assert { type: "javascript" };

In #49 we discussed whether these should be permitted. They are current not permitted, but if we decide to allow them in the future then that decision should be made within EcmaScript rather than by hosts. This is enforced by the requirement that these values are not present in the host's implementation of HostGetSupportedExtraModuleScriptTypes.

Alternatively we could name the HostGetSupportedModuleScriptTypes and require the host to state whether it supports JavaScript modules. But what would it mean for a host not to support JavaScript modules, if it contains module imports? We avoid the question altogether If we use HostGetSupportedExtraModuleScriptTypes and only require hosts to specify module types other than JavaScript.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated
<ul>
<li>It must return a List whose values are all StringValues, each indicating a supported module script type.</li>

<li>None of the string values may be *"js"* or *"javascript"*.<emu-note type="editor">It is assumed that Source Text Module Record must be supported.</emu-note></li>
Copy link
Member

@ljharb ljharb Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like it prohibits hosts from making an optional “javascript” or “js” type, but doesn’t actually prevent them from supporting or not supporting source text module records, or any other JavaScript-representing type under another name. What’s the benefit to this constraint?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is to enforce that neither of the following can be used to import a source text module record:

import "./foo.js" assert { type: "js" };
import "./foo.js" assert { type: "javascript" };

There was some discussion in #49 about whether or not those should be permitted. It seems to me that the decision of whether or not they work or not should be up to ECMAScript, not the individual hosts. ECMAScript can choose to allow them or disallow them by editing whether they are restricted in this <li>, and/or adding them to the HostGetSupportedExtraModuleScriptTypes list if the host didn't add them.

On the other hand if we want the hosts to make that call then this item can just be deleted.

Copy link
Member

@ljharb ljharb Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current text of this PR does not prohibit a host from doing:

import "./foo.js" assert { type: "ecmascript" };
import "./foo.js" assert { type: "jscript" };
import "./foo.js" assert { type: "JS" };
import "./foo.js" assert { type: "JavaScript" };

In other words, if the goal is to prohibit hosts from allowing a type assertion on source text-based modules, then this restriction, as written, doesn't achieve it - it only prohibits 2 of the infinite possible strings that could be used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can only make that restriction in prose, right? Similar to how we have prose restrictions on job queuing. It seems reasonable to do that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So something like "None of the string values must be used for Source Text Module Records."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I very much doubt there could be any kind of consensus for a registry; but I’m not up to speed on all the built-in module discussions on that topic.

If the spec prohibits any host-defined type value from representing STMR, but mandates a specific one be supported, that seems fine with me - but then we have to decide how to spell it :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point @littledan created a repository for host coordination I think. Perhaps we can just maintain an informal list there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb @annevk You're right, it doesn't really work to try to restrict type js/javascript/etc here. I've removed this part of the change.

Considering JSON modules, EcmaScript says that if there is type: "json" then the import must be a JSON module or fail to load, but EcmaScript takes no position on whether a host could load JSON modules using other values of the type assertion in addition to "json". So maybe it's wrong to single out Source Text Modules as needing restrictions like this.

Early thinking about a registry was included in this repo at some point but was removed so as not to draw too much focus from the core of the proposal. I'm not sure it's been discussed much since. Our best bet may indeed be to maintain an informal list like that (but where?), and ultimately leave such decisions up to the host.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think you'd want to at least reserve es/ecmascript/js/javascript, perhaps with an assertion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I finally found it, we should document these strings in @littledan's https://github.com/littledan/js-shared-interfaces.


<li>None of the string values may be *"js"* or *"javascript"*.<emu-note type="editor">It is assumed that Source Text Module Record must be supported.</emu-note></li>

<li>Each time this operation is called, it must return the same List instance with the same contents.</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per-realm, or program-wide?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we want program-wide here. I would be surprised if a host would change the module types it supports on a per-realm basis. I could be wrong about that though.

I was mimicking language I saw for things like HostHasSourceTextAvailable: "Each time it is called with a specific func as its argument, it must return the same completion record.". I guess that probably means per-realm, though?

Is there a formal way to express that something must return the same thing program-wide -- assuming that's what we actually want here?

Copy link
Member

@ljharb ljharb Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually certain here. It might be that every AO is already implicitly per-realm, which would mean this wording doesn't force it across every realm.

cc @erights @caridy @leobalter for any relation to the Realms/Compartments proposals, cc @syg @bakkot @michaelficarra for the question as it pertains to the actual spec.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for Realms, we are proposing that each Realm has a different module map, but I assume the module types mapping could be just reused. In this case, I believe it wouldn't be an issue. This mapping won't share Realm identities or any fingerprints, anyway.

I believe @erights might have a different perspective for the compartments proposal, but I can't answer for that without more context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to at least be per agent. We'd definitely not support HTML modules in workers or worklets for instance.

spec.html Outdated Show resolved Hide resolved
dandclark and others added 2 commits March 12, 2021 11:29
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
@ljharb
Copy link
Member

ljharb commented Apr 4, 2023

@nicolo-ribaudo this seems closeable since the semantics are pretty locked in?

@nicolo-ribaudo
Copy link
Member

No one is currently pushing for this PR and it has been lying around for two years, so I'm closing this PR.

However, note that the proposed behavior matches what all the current hosts do: if someone is interested, it would be possible to bring this up again in the future as a needs-consensus PR. It's a matter of deciding where to enforce this behavior (if in ECMA-262 to ensure compatibility, or if it's ok to let hosts decide since they will very likely all end up throwing on unknown types anyway).

@ljharb ljharb deleted the user/daniec/HostGetSupportedExtraModuleTypes branch April 5, 2023 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants