-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add specification for Synthetic Module Records #44
Conversation
This looks great to me. I don't see any particular technical issues, just some one editorial comment (which would not be any sort of normative change): All use cases I can think of for synthetic modules would have fixed exports once the module is "parsed". What if CreateSyntheticModule instead took two arguments, the realm and a mapping from strings to JavaScript values (or, since we don't have infra, a List of Records { [[Name]]: String, [[Value]]: JS value }), and set these up as immutable bindings during the Initialize() phase? We can consider generalizing the mechanism if there's a particular use case for it. cc @Ms2ger |
Yeah, that's a possible approach. However, my thinking went toward the current approach like so:
I'm open to changing this, but I think the current approach is more flexible, so I'm inclined toward that one. |
To be more concrete, with the JSON module example, with the OP design I think it would have straightforward evaluation steps:
But with a static-exports approach, it would instead need to be something like:
|
Thanks for walking through these error cases; that makes the motivation for this design a lot clearer. The strategy you describe for JSON parse errors is sort of what I expected (that's what we do for JS early errors phase-wise, right?), but I can see how it's a lot more complicated to write down. Actually, now, we came to an observable difference, didn't we? The find the first parse error algorithm would stop any module from executing if it hit a JSON parse error if these are treated as parse errors. But, if they are treated as errors during the execution phase, it might throw the exception after executing some other earlier sibling modules. (Maybe we should move this discussion back to whatwg/html#4315 (comment) .) All I could find on import-maps for why it throws an exception on evaluation rather than being absent from the import-map (which is what I would've expected, but don't feel strongly about) was WICG/kv-storage#16 . Was there more context somewhere, or should I file a different issue? |
This patch provides JSON modules as a single default export, with parse errors checked before instantiating the module graph. Note, editorially, it's unclear whether JSON modules should be considered a type of "module script", with a settings object, fetch options, base URL, etc or not. This patch considers them "module scripts", but leaves those record fields unset (as they are unused). This patch is based on tc39/proposal-built-in-modules#44 which hasn't landed yet, so the references are a bit awkward, and this patch should not land until that one does. Closes whatwg#4315
Thinking about this more: Even if I am not sure about those semantic details for the use cases, I think it's good to give module evaluation steps, to do the equivalent of executing class declarations, etc. This would make it very practical to have a real implementation which executes synthetic modules based on JavaScript modules. So, +1 to all of this PR from me. From that frame, I'm wondering: Should we leave the step of initializing bindings to the module, rather than initializing them to undefined? |
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
This patch provides CSS modules as a single default export, of a CSSStyleSheet object, which is not added to the document. Edge cases which might be worth reconsidering: - @imports are recursively fetched together with the module graph, blocking script execution. Network errors reached prevent the execution of the entire module graph. - Any MIME type whose essence is "text/css" is accepted; this appears to be weaker checking than elsewhere in the specification. - Although the Constructable Stylesheet Objects proposal is used for infrastructure, the resulting CSSStyleSheet object acts as if it were not constructed (i.e., you can't call the replace() method). This patch is based on tc39/proposal-built-in-modules#44 Closes whatwg#4315
This patch provides CSS modules as a single default export, of a CSSStyleSheet object, which is not added to the document. Edge cases which I didn't see discussed elsewhere: - @imports are recursively fetched together with the module graph, blocking script execution. Network errors reached prevent the execution of the entire module graph. - Any MIME type whose essence is "text/css" is accepted; this appears to be weaker checking than elsewhere in the specification. - Although the Constructable Stylesheet Objects proposal is used for infrastructure, the resulting CSSStyleSheet object acts as if it were not constructed (i.e., you can't call the replace() method). Note, the Constructable Stylesheet Objects proposal makes important steps to specifying loading of @import, but there may still be room for more precise plumbing with HTML. This text ensures that style sheet module scripts have a base URL and fetch options, which might be referenced by the definition of @import in the future. This patch is based on tc39/proposal-built-in-modules#44 Closes whatwg#4315 Closes WICG/webcomponents#759
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
[[Status]] is a field of Cyclic Module Records; Abstract Module Records don't keep track of it. This patch avoids checking the [[Status]] of modules that don't have one, instead first checking whether the module is cyclic. All of the use cases seemed to be when modules were in dependency chains and not leaves. Given the proposed Synthetic Module Records tc39/proposal-built-in-modules#44 and their possible usage in JSON modules, CSS modules, and WebIDL modules, it makes sense to avoid these code paths for those cases.
1. Set the LexicalEnvironment of _moduleCxt_ to _module_.[[Environment]]. | ||
1. Suspend the currently running execution context. | ||
1. Push _moduleCxt_ on to the execution context stack; _moduleCxt_ is now the running execution context. | ||
1. Let _result_ be the result of performing ? _module_.[[EvaluationSteps]](_module_). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this ?
be here? It seems like the execution context stack will be messed up if this throws.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
seems right to me--the module evaluation algorithms are all written around Evaluate()
maybe throwing. See tc39/ecma262#916 for more context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, step 5 of https://tc39.github.io/ecma262/#sec-moduleevaluation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @Ms2ger is right. The ? is wrong; the exception gets re-thrown (or the normal completion gets returned) 3 steps down, but before doing that we need to clean up the execution context stack stuff.
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
[[Status]] is a field of Cyclic Module Records; Abstract Module Records don't keep track of it. This patch avoids checking the [[Status]] of modules that don't have one, instead first checking whether the module is cyclic. All of the use cases seemed to be when modules were in dependency chains and not leaves. Given the proposed Synthetic Module Records tc39/proposal-built-in-modules#44 and their possible usage in JSON modules, CSS modules, and WebIDL modules, it makes sense to avoid these code paths for those cases.
[[Status]] is a field of Cyclic Module Records; Abstract Module Records don't keep track of it. This patch avoids checking the [[Status]] of modules that don't have one, instead first checking whether the module is cyclic. All of the use cases seemed to be when modules were in dependency chains and not leaves. Given the proposed Synthetic Module Records tc39/proposal-built-in-modules#44 and their possible usage in JSON modules, CSS modules, and WebIDL modules, it makes sense to avoid these code paths for those cases.
This patch provides JSON modules as a single default export, with parse errors checked before instantiating the module graph. Note, editorially, it's unclear whether JSON modules should be considered a type of "module script", with a settings object, fetch options, base URL, etc or not. This patch considers them "module scripts", but leaves those record fields unset (as they are unused). This patch is based on tc39/proposal-built-in-modules#44 which hasn't landed yet, so the references are a bit awkward, and this patch should not land until that one does. Closes whatwg#4315
Hey folks, I'm planning to look into adding this text to WebIDL for the time being, so we can move forward with whatwg/html#4407. I'd be happy to help with moving this into Ecma262 or any other place in the future, if that's what we decide on. I'd certainly want to make sure it remains useful for the work you're doing here. |
Based on text written by Domenic at <tc39/proposal-built-in-modules#44>.
Please see whatwg/webidl#722 |
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
Based on text written by Domenic at tc39/proposal-built-in-modules#44. These will be used by HTML in whatwg/html#4407.
This patch provides CSS modules as a single default export, of a CSSStyleSheet object, which is not added to the document. Edge cases which I didn't see discussed elsewhere: - @imports are recursively fetched together with the module graph, blocking script execution. Network errors reached prevent the execution of the entire module graph. - Any MIME type whose essence is "text/css" is accepted; this appears to be weaker checking than elsewhere in the specification. - Although the Constructable Stylesheet Objects proposal is used for infrastructure, the resulting CSSStyleSheet object acts as if it were not constructed (i.e., you can't call the replace() method). Note, the Constructable Stylesheet Objects proposal makes important steps to specifying loading of @import, but there may still be room for more precise plumbing with HTML. This text ensures that style sheet module scripts have a base URL and fetch options, which might be referenced by the definition of @import in the future. This patch is based on tc39/proposal-built-in-modules#44 Closes whatwg#4315 Closes WICG/webcomponents#759
This PR defines the ability in WebIDL to define JavaScript built-in modules, 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/
Hey folks!
I thought I'd get things started with a spec for module records that seem like they'd be useful for built-in modules, JSON modules, CSS modules, etc. As we work through these ideas on the web platform side, we've found some confusions that would be prevented with a more concrete spec text, so I wanted to write something up we could point to.
I've deployed a preview of the built product at https://proposal-javascript-standard-library-domenic.now.sh/. Thoughts welcome!