Define Web IDL Modules

[littledan]

This PR defines the ability in WebIDL to define JavaScript built-in modules.

Modules are defined with a new module declaration, which can contain
regular operations, readonly attributes, and interfaces. For example:

[Exposed=Window, SecureContext]
module temporal {
  interface Timezone {
    readonly attribute USVString name;
    long long offsetMs(long long unixTime);
  };
  temporal.Timezone getCurrentTimezone();
  readonly attribute temporal.Timezone initialTimezone;
};

This PR is based on the proposed infrastructure at
tc39/proposal-built-in-modules#44
These semantics set the module map, which makes sense in the context of
the import maps draft specification
https://wicg.github.io/import-maps/

Closes #592

---

Preview | Diff

[littledan]

This came out much simpler than I expected, due to @domenic 's great work setting up related specifications. I'm not sure if this handles partials and overloaded operations properly, or if it does enough to prohibit things that don't make sense; please consider this an early draft. cc @Ms2ger

[annevk]

A thing I'm missing here is infrastructure around scoping.

module X {
  interface Y { ... };
};
interface Y { ... };

Is that okay? If not, what forbids it? If it is, what forbids the second interface declaration from being partial interface Y? (Or maybe this why [Exposed] is not allowed?)

[littledan]

I was hoping that this would be OK to have two different interfaces named Y, one inside a module and one not. I hope modules will let us scale up spec development exactly for this kind of reason. In drafting this text, I was having trouble finding what I would have to change/clarify to make sure that a later partial interface Y wouldn't merge with the interface in the module.

[Exposed] and [SecureContext] are not allowed just because it'd be a bunch of extra complexity:

• On the spec side: if you have partial module "std:foo" { partial interface Bar { void method(); } }, you'd have to check five things to understand how the exposure-related extended attributes apply to method: the main module, the partial module, the main interface, the partial interface, and the operation. We could specify this, but it'd be a bit more complicated, and not simply the current algorithm of (roughly) "traverse up until you find the attribute".
• On the implementation side: if you write a built-in module in JavaScript, you'd have to provide an entirely different version to omit certain named exports, as the list of exported names is static and indicated in the JavaScript code. And if you use a JavaScript class, you'd have to mutate the result to update visibility.
• On use cases: I think modules will be a bit more granular than some of the mega-interfaces that the Web has been evolving over time. So I think that these attributes make less sense for things inside of modules.

If you think full expressiveness here is important, I could update this patch to do that.

[annevk]

That all sounds good to me, modulo what comes out of #676 but that can be addressed at that point and isn't needed for v1. (I'm not entirely sure what needs changing to clarify scope unfortunately.)

[Ms2ger]

Some initial thoughts, mostly editorial, below.

[bzbarsky]

@littledan Is there a summary somewhere of what the actual syntax this PR implements is?

[littledan]

@bzbarsky Added an example in the PR description; is this the level of detail you're looking for?

[domenic]

Very nice. Overall LGTM, although maybe we should hold off on merging until we get synthetic module records accepted.

[littledan]

Thinking about the comments on ambiguity from @domenic and @annevk a bit more, I'm thinking that it could be best to switch to a model where you declare a module with an identifier and then refer to interfaces declared within it as moduleIdentifer.Interface (whether you're inside or outside of a module). It'd be really complicated to do lookup based on location, and hurt expressiveness.

The only thing I can see potentially missing is that you can't use / within an identifier. This can be addressed in a few different possible ways (allowing identifiers to contain / in general, or letting module identifers have this special right) which we could address when someone proposes a built-in module which contains / in its module specifier.

It'll be a little while until I have a chance to get back to this; if @Ms2ger or someone else is interested in picking this up, that'd be welcome, or otherwise I will a bit later.

[Ms2ger]

This is ready for review, btw.

[littledan]

A couple additional suggestions which @domenic and I recently discussed offline. I think any of these could be follow-ons and don't need to block the PR.

• This is a new feature that's not quite ready to use, given surrounding ongoing specification efforts. There should probably be a note to this effect, to avoid confusing people.

• Maybe we should use export rather than readonly attribute as the syntax?

• We could consider moving the synthetic module record definition into WebIDL, being explicit that it could move back into the ES spec at some point later if there is interest (in a note?). Editorially, we could also consider defining synthetic module records' contents during "create a synthetic module record", rather than during the evaluate phase steps (either way this goes, we could include a note explaining that there are multiple implementation options, both literally lazy-loading JS code and also setting them up more statically).

[domenic]

Overall LGTM. I think we should add an "under construction" signage, file issues for the other follow-ups @littledan mentioned, and merge. Really nice work.

We can then iterate on those issues and also attempt to use modules in the KV storage spec. I don't think we'll want to merge into KV storage quite yet because of the remaining issues in WICG/kv-storage#46, but just writing the spec patch would give us a good idea of how these things would be used. For example it'd let us see "set a module attribute" in action.

[domenic]

Why is this a separate concept from qualfiied name? The difference seems to be that for modules we allow two same-named interfaces in different modules, but we don't allow two same-named interfaces in [LegacyNamespace]. Is that mismatch desirable?

[Ms2ger]

Changing how LegacyNamespace works in this PR seems out of scope to me. Note that, if I connected everything correctly, using the qualified name would mean you'd need changes like this to specifications that use it:

 namespace WebAssembly {
-    Promise<Module> compile(BufferSource bytes);
+    Promise<WebAssembly.Module> compile(BufferSource bytes);
 };

Maybe we want that, but let's discuss that separately.

[domenic]

Just want to check that this is not blocked on me at this point. I'm excited to see this land, but I also appreciate how every new commit from @Ms2ger corrects more subtle issues that I totally missed, so I'm happy to wait it out.

[annevk]

Things that would be nice to see clarified:

• What's the plan for testing?
• What are implementers planning? Browsers as well as the various tools that will need updates.

[Ms2ger]

What's the plan for testing?

Wait until someone uses the feature, and then write tests. Possibly https://tc39.github.io/proposal-temporal/ or https://wicg.github.io/kv-storage/ might be the first.

What are implementers planning? Browsers as well as the various tools that will need updates.

I don't have a clue. Maybe @saschanaz has thoughts on webidl.js.

[Ms2ger]

I think I'm happy to squash and land this. (At least until the next time I read through the diff.) I'll do that some time next week, if I hear no objections.

[saschanaz]

Maybe @saschanaz has thoughts on webidl.js.

Will happily start to add support when this PR merges.

[annevk]

I'm a little worried about landing something this big without a downstream user PR and browser implementation commitments. We kinda stopped adding "speculative" things a while ago. And while this isn't completely speculative, it still seems good to work it out end-to-end, no?

[saschanaz]

Why do we have two exactly same grammars Module and PartialModule?

[Ms2ger]

To distinguish the two cases in the definition of "module declaration" and "partial module declaration". There may be a better way to do this.

[saschanaz]

I think the following two sentences are ambiguous become the grammars are same:

• A module declaration is a definition matching the Module nonterminal symbol.
• A partial module declaration is a definition matching the PartialModule nonterminal symbol.

Ah okay, actually not, sorry.

[littledan]

I think I'm happy to squash and land this.

I'd be happy to see this land, but could you make a big "work in progress within the draft, do not use yet" at the top? @domenic and I have been talking about whether interfaces should switch into some "new defaults" when inside of a module, which would bring sizable semantics changes.

[littledan]

+1

[littledan]

cc @mattijs

[domenic]

@annevk, are you OK with landing this? Chrome is interested in implementing, and my understanding is that folks like @lukewagner and @tschneidereit were also interested. (@Ms2ger, there are some merge conflicts.)

[annevk]

Is there a downstream PR? What about the sizeable semantic changes @littledan mentioned?

[domenic]

I'm unsure what you mean by "downstream"?

We indeed might make sizable semantics changes before shipping; those are captured in e.g. #718 and #719. But it's good to make incremental progress, so that specs can use modules at all, before we contemplate exactly what might change by default inside modules.

[annevk]

I meant a PR for a spec that wants to use this as-is.

[domenic]

Ah, I'll work on that today.

[domenic]

For posterity, @Ms2ger notes that he's already started in https://github.com/WICG/kv-storage/compare/master...Ms2ger:idl?expand=1, and in fact the modules part is done there. (It appears that parts that are unfinished are related to #720.)

[domenic]

I think we should be able to land this after rebasing. WICG/kv-storage#68 is what wants to use this.

[domenic]

@annevk ping on whether you're OK to land this. I am going to land WICG/kv-storage#68 today with reference to this pull request, but basing it on the actual IDL spec would be even better.

[annevk]

For clarity, while Mozilla is supportive of KV Storage, it's still a little unclear what is happening with Builtin Modules (see mozilla/standards-positions#147) which means I'm not comfortable moving forward with this.

[littledan]

Given the current state of built-in modules in TC39, I think we're not ready for WebIDL support for built-in modules yet. I'm going to close this PR and defer to the efforts of @msaboff @mattijs and others, to build the foundation before we consider higher-level aspects.