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

HTML modules spec draft editorial feedback #783

Open
domenic opened this issue Jan 17, 2019 · 14 comments

Comments

Projects
None yet
4 participants
@domenic
Copy link
Contributor

commented Jan 17, 2019

Hey @dandclark, @BoCupp, @samsebree, @travisleithead. Excited to see the blink-dev intent to implement! I wanted to provide some spec feedback on https://github.com/w3c/webcomponents/blob/gh-pages/proposals/html-module-spec-changes.md in the hopes of making the future integration into HTML smoother. It's all generally editorial, but I thought getting it out there sooner would be a good idea.

  • We should define the new module record type in the HTML spec, not the ES spec. ES provides a base class, which gets specialized by different host environments which have their own needs---e.g. the WebAssembly spec will have its own type of module record, and HTML modules should have theirs. We shouldn't bake HTML-specific semantics into the core language.
  • We should rebase the spec on top of tc39/ecma262#1311. I am not sure why that hasn't landed yet, but I've asked. That should make things easier in various ways.
  • Since HTML modules will continue to be included through <script type="module">, we should continue using the script infrastructure. As such, just as a naming thing, the spec-level struct should be "HTML module script", which is a third type of script. We should probably then rename the existing "module script" type to "JS module script"; that will be needed anyway when we introduce WebAssembly module scripts.
  • [[ScriptName]] is a confusing slot name; I'd suggest [[ExternalScriptURL]]. Then maybe [[ModuleRecord]] could be named [[InlineModuleRecord]]. You could also probably get rid of [[IsInline]], just keying off the null-ness or not of those other two slots.

Overall, very exciting stuff!

@domenic domenic added the modules label Jan 17, 2019

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2019

This is all great feedback, thanks! I'll work on getting these changes incorporated into the docs.

@annevk

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

Off-topic: @dandclark there's also other issues labeled "modules" with feedback that doesn't appear addressed (e.g., the suggested MIME type is text/html iiuc).

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

Thanks @annevk -- I'll make sure we get to these too.

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2019

@domenic,

For your third bullet point, what do you think about HTML Module Script/JS Module Script (and eventually JSON Module Script etc) types sharing a common abstract Module Script base type?
That is, do we want type hierarchy #⁠1:
Classic Script is a Script
Module Script is a Script
JS Module Script is a Module Script
HTML Module Script is a Module Script

Or #⁠2:
Classic Script is a Script
JS Module Script is a Script
HTML Module Script is a Script

Given that the current Module Script struct doesn't have any items in addition to Script, there isn't extra state that a common Module Script base type would need to include. However, for some algorithms it seems to me there is value in having a common "Module Script" type. For example consider Fetch a module script graph. If there is no abstract Module Script type, what could this algorith return? It would have to be a Script or a JS Module Script/HTML Module/Script union -- either of which don't seeem great. So I'm leaning towards type hierarchy #⁠1 for this reason.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

Hmm, I think a union is fine. I don't see any problems that would be caused by it, especially in terms of the spec. Do you?

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2019

No technical problems really, my thinking was more that it might not scale well if other module types are introduced as is being discussed elsewhere. We could see the spec peppered with stuff like "the algorithm will asynchronously complete with null or a JS Module Script, or an HTML Module Script, or a JSON Module Script, or a CSS Module Script...". I've been poking around the spec for some sort of precedent here but haven't come up with anything.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2019

I mean, you can just say "this algorithm will asynchronously complete with null or a module script", where "module script" is defined as a union of all those other things.

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2019

Yeah, that makes sense. Thanks.

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

I've pushed an update to the proposed spec changes doc that addresses most of this feedback as well as other improvements and corrections. The description for the PR outlines the highlights.

The biggest thing not yet addressed is the move of HTML Module Record's definition to the HTML5 spec. It's not yet clear to me what this cross-spec inheritence should look like, or exactly what the final API surface for each spec should be. However I've tried to push all the HTML-specific stuff over to that spec (knowledge of documents, inline/external scripts, etc). A key bit of this is that the construction of HTML Module Records is all on the HTML side since that requires knowledge of documents and script elements. ES just uses the finished structure that's passed over, whose only indication that it knows anything about HTML DOM is in the name ScriptEntry, which should probably be changed; I haven't thought of something better yet.

Thanks for reviewing the documents!

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

Hmm, it's pretty clear to me that HTML Module Records belong in the HTML spec. TC39 would be very unlikely to accept them into their spec, and indeed has recently contemplated moving Source Text Module Records out of ecma262 and into HTML, where they'd be renamed "Web JavaScript Module Records". (Basically because they don't work very well for Node.js.) Similarly, for integration with WebAssembly, the WebAssembly Module Record is being defined in the WebAssembly spec.

Is there any counterargument for why this would belong in the base JS language spec?

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

I don't have any particular objection here, I just haven't made the change yet because there are still some aspects of it I'm pondering. For example, what does HTML Module Record's InitializeEnvironment() look like when it lives in the HTML spec? Is it allowed to just call ES algorithms like CreateMutableBinding() (to prepare the module's default export) or does there need to be some special handoff between the specs for stuff like that? If so, what should that look like? I hadn't yet seen the WebAssembly ESM integration spec draft but this looks like it will be a helpful reference so I'm reading it now.

Is the discussion about moving Source Text Module Records into HTML a public one that you could link me to? I'd be interested in following that.

@domenic

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

For example, what does HTML Module Record's InitializeEnvironment() look like when it lives in the HTML spec? Is it allowed to just call ES algorithms like CreateMutableBinding() (to prepare the module's default export)

Yep, we call ES abstract ops pretty frequently from the HTML spec; for example see https://html.spec.whatwg.org/#structuredserializeinternal or the other things listed in the JavaScript section of https://html.spec.whatwg.org/#dependencies

Is the discussion about moving Source Text Module Records into HTML a public one that you could link me to? I'd be interested in following that.

It was first mentioned in tc39/ecma262#1306 (precisely in tc39/ecma262#1306 (comment)) and then discussed more in the latest TC39 face to face meeting, minuted in https://github.com/tc39/tc39-notes/blob/master/es9/2019-01/jan-30.md#dynamic-modules-layering

@dandclark

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

Thanks for the links! I have a PR here that pulls all the changes over to the HTM5 spec, along with some other minor adjustments (e.g. adding hyperlinks for various calls that are now cross-spec).

@travisleithead

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

Anything further here? Or shall we just close this? (We can always add new issues later of course!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.