-
Notifications
You must be signed in to change notification settings - Fork 164
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
Define Synthetic Module Records #722
Conversation
Based on text written by Domenic at <tc39/proposal-built-in-modules#44>.
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.
LGTM with nits. I will just add a commit fixing the nits, actually.
Pushed my fixes. @Ms2ger to confirm they look good, and we can leave this up for a day or so if other folks want to take a look. |
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.
Your changes look good, thanks!
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.
LGTM with stylistic suggestion that we could get back to later.
<h4 id="createsyntheticmodule">CreateSyntheticModule</h4> | ||
|
||
<div algorithm> | ||
The abstract operation <dfn abstract-op>CreateSyntheticModule</dfn>(|exportNames|, |evaluationSteps|, |realm|, |hostDefined|) creates a [=Synthetic Module Record=] based upon |
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.
Since we are in WebIDL, I wonder if Create SyntheticModule and SetSyntheticModuleExport would be more clean if phrased as worded algorithms, e.g., "To create a synthetic module record...". HostDefined also seems redundant here when the steps can close over outer variables (as they can in web specs, but not JS).
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'd prefer to keep this close to ecma-speak for now, in case we decide to move it back under the TC39 umbrella.
This commit adds JSON modules as a single default export, with parse errors checked before instantiating the module graph. As infrastructure, this divides the "module script" concept into "JavaScript module scripts" and "JSON module scripts". Most of the spec's existing uses of "module script" become "JavaScript module script". JSON module scripts are fetched in the same way as JavaScript module scripts, e.g. with the "cors" mode and using strict MIME type checking. They use the Synthetic Module Record defined in whatwg/webidl#722. Closes #4315. Closes WICG/webcomponents#770.
Based on text written by Domenic at
tc39/proposal-built-in-modules#44.
Preview | Diff