Normative: Add JSON modules #3391

nicolo-ribaudo · 2024-08-14T11:58:49Z

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.

nicolo-ribaudo · 2024-08-14T12:01:00Z

spec.html

+
+        <emu-clause id="sec-createsyntheticmodule" type="abstract operation">
+          <h1>
+            CreateSyntheticModule (


This AO is only used once, it's not used in any host spec, and its implementation is a single line. Can we inline it?

It seems like V8 has an API based on this AO (https://v8docs.nodesource.com/node-13.2/df/d74/classv8_1_1_module.html#aeaa8c3cfb75478b9403be3d3955bb338), but nothing prevents implementations to keep providing this sort of constructor for Synthetic Module Records.

Yeah I would inline this and remove the references to it.

I'm fine with inlining it.

spec.html

nicolo-ribaudo · 2024-08-14T12:02:38Z

spec.html

+        </emu-clause>
+
+        <emu-clause id="sec-smr-module-record-methods">
+          <h1>Implementation of Module Record abstract methods</h1>


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.

Sure, they should match.

nicolo-ribaudo · 2024-08-14T12:04:03Z

spec.html

+
+          <p>The following are the concrete methods for Synthetic Module Record that implement the corresponding Module Record abstract methods defined in <emu-xref href="#table-abstract-methods-of-module-records"></emu-xref>.</p>
+
+          <emu-clause id="sec-smr-LoadRequestedModules" type="concrete method">


There is an open PR in the proposal changing these smr shorthands to the longer sec-synthetic-module-record-LoadRequestedModules. What is the editorial preference?

I am completely unopinionated on IDs. This ID is fine.

nicolo-ribaudo · 2024-08-14T12:06:52Z

spec.html

+          </dl>
+
+          <emu-alg>
+            1. Let _json_ be ? Call(%JSON.parse%, *undefined*, « _source_ »).


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

I'm fine with either way.

I mildly prefer the AO (after #3394 lands), but that refactoring can happen after this PR.

nicolo-ribaudo · 2024-09-24T04:15:38Z

@tc39/ecma262-editors Remember that this is going for stage 4 at the next meeting :)

linusg · 2024-10-08T05:28:47Z

spec.html

@@ -28495,12 +28698,19 @@ <h1>
            </ul>
            <p>and it performs FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_) where _result_ is a normal completion, then it must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_) with the same _result_ each time.</p>
          </li>
+          <li>
+            <p>If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is *"type"* and _entry_.[[Value]] is *"json"*, the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invokation of ParseJSONModule or a throw completion.</p>


Suggested change

If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is *"type"* and _entry_.[[Value]] is *"json"*, the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invokation of ParseJSONModule or a throw completion.

If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is *"type"* and _entry_.[[Value]] is *"json"*, the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invocation of ParseJSONModule or a throw completion.

michaelficarra · 2024-10-23T02:18:03Z

spec.html

          <li>
            The operation must treat _payload_ as an opaque value to be passed through to FinishLoadingImportedModule.
          </li>
        </ul>

        <p>The actual process performed is host-defined, but typically consists of performing whatever I/O operations are necessary to load the appropriate Module Record. Multiple different (_referrer_, _moduleRequest_.[[Specifier]], _moduleRequest_.[[Attributes]]) triples may map to the same Module Record instance. The actual mapping semantics is host-defined but typically a normalization process is applied to _specifier_ as part of the mapping process. A typical normalization process would include actions such as expansion of relative and abbreviated path specifiers.</p>
+
+        <emu-note>
+          <p>The above text implies that hosts *must* support JSON modules imported with `type: "json"` (if it completes normally), but it doesn't prohibit hosts from supporting JSON modules imported with no type specified.</p>


Suggested change

The above text implies that hosts *must* support JSON modules imported with `type: "json"` (if it completes normally), but it doesn't prohibit hosts from supporting JSON modules imported with no type specified.

The above text requires that hosts support JSON modules when imported with `type: "json"` (and HostLoadImportedModule completes normally), but it does not prohibit hosts from supporting JSON modules when imported without `type: "json"`.

michaelficarra

LGTM otherwise.

syg · 2025-01-22T21:57:18Z

spec.html

+
+        <p>A <dfn variants="Synthetic Module Records">Synthetic Module Record</dfn> is used to represent information about a module that is defined by specifications. Its exported names are statically defined at creation as an argument to CreateSyntheticModule, while their corresponding values can change over time using SetSyntheticModuleExport. It has no imports or dependencies.</p>
+
+        <emu-note>A Synthetic Module Record could be used for defining a variety of module types: for example, built-in modules, or JSON modules, or CSS modules.</emu-note>


Suggested change

<emu-note>A Synthetic Module Record could be used for defining a variety of module types: for example, built-in modules, or JSON modules, or CSS modules.</emu-note>

<emu-note>A Synthetic Module Record could be used for defining a variety of module types: for example, JSON modules or CSS modules.</emu-note>

I would limit the examples to ones that exist today.

syg · 2025-01-22T22:01:24Z

spec.html

+            <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>


Suggested change

<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>

<td>The initialization logic to perform upon evaluation of the module, taking the Synthetic Module Record as its sole argument. It must not modify [[ExportNames]]. It may return an abrupt completion.</td>

That sentence seems superfluous to me.

syg · 2025-01-22T22:02:05Z

spec.html

+
+        <emu-clause id="sec-createsyntheticmodule" type="abstract operation">
+          <h1>
+            CreateSyntheticModule (


I'm fine with inlining it.

syg · 2025-01-22T22:10:25Z

spec.html

+          </h1>
+          <dl class="header">
+            <dt>description</dt>
+            <dd>It can be used to set or change the exported value for a pre-established export of a Synthetic Module Record.</dd>


Suggested change

<dd>It can be used to set or change the exported value for a pre-established export of a Synthetic Module Record.</dd>

<dd>It can be used to set or change the exported value for an existing export of a Synthetic Module Record.</dd>

syg · 2025-01-22T22:27:39Z

spec.html

@@ -28495,12 +28698,19 @@ <h1>
            </ul>
            <p>and it performs FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_) where _result_ is a normal completion, then it must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_) with the same _result_ each time.</p>
          </li>
+          <li>
+            <p>If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is *"type"* and _entry_.[[Value]] is *"json"*, the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invokation of ParseJSONModule or a throw completion.</p>


I find writing this as a normative requirement on a host hook roundabout. I see two callsites to HostLoadImportedModule, one in dynamic import and one in static import. WDYT about refactoring those callsites to directly handle JSON modules?

@nicolo-ribaudo is this easy to do without significant refactoring? If not, it's fine. Don't have the full machinery in my head.

bakkot · 2025-01-22T22:48:26Z

spec.html

+              1. Set the LexicalEnvironment of _moduleContext_ to _module_.[[Environment]].
+              1. Suspend the currently running execution context.
+              1. Push _moduleContext_ on to the execution context stack; _moduleContext_ is now the running execution context.
+              1. Let _steps_ be _module_.[[EvaluationSteps]].


If we're going to put the result of [[EvaluationSteps]] into a promise, as below, then it needs to be constrained to actually return an ES value. Right now there is no such constraint, and the only actual use in the spec is an Abstract Closure which doesn't return anything.

My inclination is to say that in the case of a normal completion the value is not used, and instead we put undefined in the Promise.

bakkot · 2025-01-22T22:48:50Z

spec.html

+            <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>


What's the use case for returning an abrupt completion?

nicolo-ribaudo commented Aug 14, 2024

View reviewed changes

nicolo-ribaudo added pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4. labels Aug 14, 2024

nicolo-ribaudo changed the title ~~Add Import Attributes proposal~~ Normative: Add JSON modules Aug 14, 2024

nicolo-ribaudo added has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Aug 14, 2024

nicolo-ribaudo commented Aug 14, 2024

View reviewed changes

Normative: Add JSON modules

749ca87

nicolo-ribaudo force-pushed the json-modules branch from 9e016c1 to 749ca87 Compare August 14, 2024 12:11

linusg reviewed Oct 8, 2024

View reviewed changes

nicolo-ribaudo added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Oct 9, 2024

michaelficarra reviewed Oct 23, 2024

View reviewed changes

michaelficarra approved these changes Oct 23, 2024

View reviewed changes

jmdyck mentioned this pull request Oct 23, 2024

Editorial: Various changes around execution contexts #2962

Open

syg reviewed Jan 22, 2025

View reviewed changes

bakkot reviewed Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Add JSON modules #3391

Normative: Add JSON modules #3391

nicolo-ribaudo commented Aug 14, 2024

nicolo-ribaudo Aug 14, 2024

michaelficarra Oct 23, 2024

syg Jan 22, 2025

nicolo-ribaudo Aug 14, 2024

michaelficarra Oct 23, 2024

nicolo-ribaudo Aug 14, 2024

michaelficarra Oct 23, 2024

nicolo-ribaudo Aug 14, 2024 •

edited

Loading

michaelficarra Oct 23, 2024

bakkot Oct 23, 2024

nicolo-ribaudo commented Sep 24, 2024 •

edited

Loading

linusg Oct 8, 2024

michaelficarra Oct 23, 2024

michaelficarra left a comment

syg Jan 22, 2025

syg Jan 22, 2025

syg Jan 22, 2025

syg Jan 22, 2025

syg Jan 22, 2025

syg Jan 22, 2025

bakkot Jan 22, 2025

bakkot Jan 22, 2025


		<p>The following are the concrete methods for Synthetic Module Record that implement the corresponding Module Record abstract methods defined in <emu-xref href="#table-abstract-methods-of-module-records"></emu-xref>.</p>

		<emu-clause id="sec-smr-LoadRequestedModules" type="concrete method">

	<p>If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is "type" and _entry_.[[Value]] is "json", the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invokation of ParseJSONModule or a throw completion.</p>
	<p>If _moduleRequest_.[[Attributes]] has an entry _entry_ such that _entry_.[[Key]] is "type" and _entry_.[[Value]] is "json", the host environment must perform FinishLoadingImportedModule(_referrer_, _moduleRequest_, _payload_, _result_), where _result_ is either the Completion Record returned by an invocation of ParseJSONModule or a throw completion.</p>

	<p>The above text implies that hosts must support JSON modules imported with `type: "json"` (if it completes normally), but it doesn't prohibit hosts from supporting JSON modules imported with no type specified.</p>
	<p>The above text requires that hosts support JSON modules when imported with `type: "json"` (and HostLoadImportedModule completes normally), but it does not prohibit hosts from supporting JSON modules when imported without `type: "json"`.</p>


		<p>A <dfn variants="Synthetic Module Records">Synthetic Module Record</dfn> is used to represent information about a module that is defined by specifications. Its exported names are statically defined at creation as an argument to CreateSyntheticModule, while their corresponding values can change over time using SetSyntheticModuleExport. It has no imports or dependencies.</p>

		<emu-note>A Synthetic Module Record could be used for defining a variety of module types: for example, built-in modules, or JSON modules, or CSS modules.</emu-note>

	<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>
	<td>The initialization logic to perform upon evaluation of the module, taking the Synthetic Module Record as its sole argument. It must not modify [[ExportNames]]. It may return an abrupt completion.</td>

	<dd>It can be used to set or change the exported value for a pre-established export of a Synthetic Module Record.</dd>
	<dd>It can be used to set or change the exported value for an existing export of a Synthetic Module Record.</dd>

Normative: Add JSON modules #3391

Are you sure you want to change the base?

Normative: Add JSON modules #3391

Conversation

nicolo-ribaudo commented Aug 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo commented Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo Aug 14, 2024 •

edited

Loading

nicolo-ribaudo commented Sep 24, 2024 •

edited

Loading