Normative: Add JSON modules #3391
Conversation
| </emu-clause> | ||
|
|
||
| <emu-clause id="sec-smr-module-record-methods"> | ||
| <h1>Implementation of Module Record abstract methods</h1> |
There was a problem hiding this comment.
Quoting a note from the original authors of the proposal:
I find having this wrapping sub-clause cleaner and suggest we do the same for Source Text Module Records in the main spec.
There was a problem hiding this comment.
Sure, they should match.
There was a problem hiding this comment.
I'm going to open a PR to align Source Text Module Record to this, but I'll do it after that the import attributes PR is merged to avoid indentation conflicts.
nicolo-ribaudo
added
pending stage 4
nicolo-ribaudo
changed the title
nicolo-ribaudo
added
has test262 tests
normative change
spec.html
Outdated
| </dl> | ||
|
|
||
| <emu-alg> | ||
| 1. Let _json_ be ? Call(%JSON.parse%, *undefined*, « _source_ »). |
There was a problem hiding this comment.
Should steps 2-9 of JSON.parse be extracted to a new AO that we can call here, or is this ok?
Related: #1012 and https://infra.spec.whatwg.org/#parse-a-json-string-to-a-javascript-value
There was a problem hiding this comment.
I'm fine with either way.
There was a problem hiding this comment.
I mildly prefer the AO (after #3394 lands), but that refactoring can happen after this PR.
nicolo-ribaudo
force-pushed
the
json-modules
branch
from
9e016c1 to
749ca87
Compare
|
@tc39/ecma262-editors Remember that this is going for stage 4 at the next meeting :) |
nicolo-ribaudo
added
has stage 4
spec.html
Outdated
| <tr> | ||
| <td>[[EvaluationSteps]]</td> | ||
| <td>an Abstract Closure</td> | ||
| <td>The initialization logic to perform upon evaluation of the module, taking the Synthetic Module Record as its sole argument. These will usually set up the exported values, by using SetSyntheticModuleExport. It must not modify [[ExportNames]]. It may return an abrupt completion.</td> |
There was a problem hiding this comment.
What's the use case for returning an abrupt completion?
There was a problem hiding this comment.
One would be, for example, for import "./x.cjs" Node.js: this would return a throw completion when the CommonJS module throws.
Note that even if we remove this possibility, you can emulate any synthetic module record with a Source Text Module Record by dynamically creating the source:
export let ...list of names...;
[[EvaluationSteps]]();so whether it's there or not doesn't really affect host capabilities.
Both JSON modules and CSS modules always return a normal completion here.
Edit: The Node.js implementation of import "./foo.cjs" is at https://github.com/nodejs/node/blob/79f96b6e84428eb8ca5b9d4433e64bea31986628/lib/internal/modules/esm/translators.js#L200, with evaluationCallback being the [[EvaluationSteps]]. wrapModuleLoad(filename, null, isMain) is the call that actually loads/runs the module, and can throw.
nicolo-ribaudo
force-pushed
the
json-modules
branch
2 times, most recently
from
ea72bf6 to
58a7f43
Compare
|
Note: CI is failing just because this PR is based on the |
nicolo-ribaudo
force-pushed
the
json-modules
branch
from
578dc2c to
2d6d20f
Compare
bakkot
force-pushed
the
import-attributes
branch
from
282c013 to
59c29bb
Compare
nicolo-ribaudo
force-pushed
the
json-modules
branch
from
2d6d20f to
f074b63
Compare
bakkot
added
the
ready to merge
This pull request builds on top of #3057.
I mostly ported the spec text as-is, taking care of integrating editorial changes from the PRs open in the repo, but I have a few comments / questions for the editors.