Layering: Enable cyclic dependencies with non-STMR module types #1311

linclark · 2018-09-24T00:35:51Z

Currently, only Source Text Module Records (STMR) can participate in cycles. From my conversations with those who put that limitation in the spec, this restriction was only added because there wasn't an immediate need for cyclic dependencies with non-STMR modules.

This PR makes it possible for other languages to add module record types that can participate in cycles with Source Text Module Records. To do this, it moves the logic for managing cycles to a new module type, Cyclic Module Record (CMR). STMR are a subclass of the CMR, and other specs such as WebAssembly can also subclass this module record type.

To help with review, I've broken this into smaller commits, but can squash them before merge if that is preferred.

Note: This is an update to #1199. In this version, I removed the concept of "subphases" that was introduced there because they are no longer needed.

jmdyck · 2018-09-24T02:15:15Z

spec.html

- <emu-alg>
- 1. For each ExportEntry Record _e_ in _module_.[[IndirectExportEntries]], do
- 1. Let _resolution_ be ? _module_.ResolveExport(_e_.[[ExportName]], &laquo; &raquo;).
+ 1. 1. _module_ be this Source Text Module Record.


"1." -> "Let"

jmdyck · 2018-09-24T02:45:35Z

spec.html

@@ -21337,8 +21337,7 @@ <h1>Static Semantics: VarScopedDeclarations</h1>
 <emu-clause id="sec-abstract-module-records">
 <h1>Abstract Module Records</h1>
 <p>A <dfn>Module Record</dfn> encapsulates structural information about the imports and exports of a single module. This information is used to link the imports and exports of sets of connected modules. A Module Record includes four fields that are only used when evaluating a module.</p>
- <p>For specification purposes Module Record values are values of the Record specification type and can be thought of as existing in a simple object-oriented hierarchy where Module Record is an abstract class with concrete subclasses. This specification only defines a single Module Record concrete subclass named Source Text Module Record. Other specifications and implementations may define additional Module Record subclasses corresponding to alternative module definition facilities that they defined.</p>
- <p>Module Record defines the fields listed in <emu-xref href="#table-36"></emu-xref>. All Module Definition subclasses include at least those fields. Module Record also defines the abstract method list in <emu-xref href="#table-37"></emu-xref>. All Module definition subclasses must provide concrete implementations of these abstract methods.</p>
+ <p>For specification purposes Module Record values are values of the Record specification type and can be thought of as existing in a simple object-oriented hierarchy where Module Record is an abstract class with both abstract and concrete subclasses. This specification defines the abstract subclass named Cyclic Module Record and its concrete subclass named Source Text Module Record. Other specifications and implementations may define additional Module Record subclasses corresponding to alternative module definition facilities that they defined.</p> <p>Module Record defines the fields listed in <emu-xref href="#table-36"></emu-xref>. All Module Definition subclasses include at least those fields. Module Record also defines the abstract method list in <emu-xref href="#table-37"></emu-xref>. All Module definition subclasses must provide concrete implementations of these abstract methods.</p>


There should be a linebreak after the first </p>.

spec.html

jmdyck · 2018-09-24T15:31:45Z

spec.html

- <emu-clause id="sec-moduledeclarationinstantiation">
- <h1>Instantiate ( ) Concrete Method</h1>
+ <emu-clause id="sec-moduledeclarationenvironmentsetup" aoid="ModuleDeclarationEnvironmentSetup">
+ <h1>ModuleDeclarationEnvironmentSetup ( )</h1>


Clause header should end with "Concrete Method".

jmdyck · 2018-09-24T15:43:32Z

spec.html

 </emu-clause>
+
+ <emu-clause id="sec-moduleexecution" aoid="ModuleExecution">
+ <h1>ModuleExecution ( )</h1>


Clause header should end with "Concrete Method".

jmdyck · 2018-09-24T15:52:07Z

spec.html


- <p>The Instantiate concrete method of a Source Text Module Record implements the corresponding Module Record abstract method.</p>
- <p>On success, Instantiate transitions this module's [[Status]] from `"uninstantiated"` to `"instantiated"`. On failure, an exception is thrown and this module's [[Status]] remains `"uninstantiated"`.</p>
+ <p>The ModuleDeclarationEnvironmentSetup concrete method implements the corresponding Cyclic Module Record abstract method.</p>


After "concrete method", insert "of a Source Text Module Record".

jmdyck · 2018-09-24T15:52:28Z

spec.html

-
- <p>The Evaluate concrete method of a Source Text Module Record implements the corresponding Module Record abstract method.</p>
- <p>Evaluate transitions this module's [[Status]] from `"instantiated"` to `"evaluated"`.</p>
+ <p>The ModuleExecution concrete method implements the corresponding Cyclic Module Record abstract method.</p>


After "concrete method", insert "of a Source Text Module Record".

littledan · 2018-10-12T11:20:15Z

This PR LGTM, modulo the nits from @jmdyck. Let's discuss this at the November 2018 TC39 meeting.

spec.html

littledan · 2018-10-19T16:41:33Z

PR to address the issues noted above: linclark#1

littledan · 2018-10-27T17:28:30Z

BTW, as this is a layering change (and not normative), and all concerns raised have been addressed, do we need to discuss it in a TC39 meeting, or can it be merged without that? Asking just to know whether I should prepare a presentation for November.

zenparsing · 2018-12-13T18:55:34Z

spec.html

@@ -21339,7 +21339,7 @@ <h1>Static Semantics: VarScopedDeclarations</h1>
 <emu-clause id="sec-abstract-module-records">
 <h1>Abstract Module Records</h1>
 <p>A <dfn>Module Record</dfn> encapsulates structural information about the imports and exports of a single module. This information is used to link the imports and exports of sets of connected modules. A Module Record includes four fields that are only used when evaluating a module.</p>
- <p>For specification purposes Module Record values are values of the Record specification type and can be thought of as existing in a simple object-oriented hierarchy where Module Record is an abstract class with concrete subclasses. This specification only defines a single Module Record concrete subclass named Source Text Module Record. Other specifications and implementations may define additional Module Record subclasses corresponding to alternative module definition facilities that they defined.</p>
+ <p>For specification purposes Module Record values are values of the Record specification type and can be thought of as existing in a simple object-oriented hierarchy where Module Record is an abstract class with both abstract and concrete subclasses. This specification defines the abstract subclass named Cyclic Module Record and its concrete subclass named Source Text Module Record. Other specifications and implementations may define additional Module Record subclasses corresponding to alternative module definition facilities that they defined.</p>


@bterlson We should correct this existing typo in a follow-up (defined => "define" or "suport").

domenic · 2019-01-17T04:39:26Z

I've noticed a lot of approved-but-not-merged PRs on this repository. This particular one is somewhat blocking to HTML modules work, and we'd love to see it land as soon as possible.

ljharb · 2019-01-17T07:48:55Z

Most of those aren't approved by all the editors yet (specifically, the editor in chief), thus they're not actually mergeable yet. I'll make sure we address this one in the next editor's meeting.

…torings.

…nt (#793) * Rebasing ES spec changes on tc39/ecma262#1311. Misc other fixes/refactorings. * Update ParseHTMLModule to populate missing fields and update HTML MR InitializeEnvironment to create *default* binding for default export of document * Add plumbing for ES spec to get the HTML Module's default export (the document) and set it to the *document* binding during module Evaluation. Plus some other minor corrections. * Update proposal to state that non-module scripts in HTML Module will cause error (rather than being silently coerced to type='module' * Minor wording/typo fixes. Eliminate disclaimer at the top about some parts being handwavy -- we've mostly progressed beyond that, I think. * Add note that parser behavior needs further specification. Change 'fetch a single module script' section to reflect that HTML Modules MIME type discussion is still ongoing.

littledan · 2019-02-16T23:19:05Z

Another month has passed. What's the process from here?

zenparsing · 2019-02-16T23:23:06Z

@bterlson This PR is labelled "es2019" but I don't know if we talked about whether you wanted to bring this one in for the 2019 branch. Thoughts?

MylesBorins · 2019-02-26T07:26:25Z

quick ping to see where this is going. It seems like cyclic module records are going to be a very useful abstraction

bterlson · 2019-02-26T16:34:32Z

Sorry for the delay, folks. This is good to go! Thanks for sticking with it.

littledan · 2019-02-28T05:47:03Z

Thanks for landing this!

This was referenced Sep 24, 2018

Normative: Support other languages in ES module graphs #1199

Closed

Layering: Rename Module.Instantiate to Module.Link #1312

Merged

jmdyck reviewed Sep 24, 2018

View reviewed changes

spec.html Show resolved Hide resolved

jmdyck reviewed Sep 24, 2018

View reviewed changes

zenparsing reviewed Oct 12, 2018

View reviewed changes

spec.html Show resolved Hide resolved

domenic mentioned this pull request Oct 22, 2018

HTML Modules WICG/webcomponents#645

Open

bterlson self-requested a review October 24, 2018 19:12

rwaldron added the has consensus This has committee consensus. label Nov 28, 2018

linclark and others added 7 commits December 12, 2018 11:52

Add Cyclic Module Record

74bdf63

Move fields related to cyclic deps from STMR to CMR

ea3997a

Add CMR method interface

eed0fe4

Refactor methods

61897e8

Rephrase cycle restriction in STMR

34eecd5

Move non-normative text about module graphs to CMR section

2464d57

Changes from code review

df6f7b3

ljharb assigned zenparsing Dec 12, 2018

ljharb force-pushed the cyclic-mr-2 branch from 022ab4a to df6f7b3 Compare December 12, 2018 19:53

bterlson requested a review from zenparsing December 12, 2018 19:53

zenparsing approved these changes Dec 13, 2018

View reviewed changes

ljharb assigned bterlson and unassigned zenparsing Dec 13, 2018

ljharb requested a review from a team December 13, 2018 23:12

domenic mentioned this pull request Jan 17, 2019

HTML modules spec draft editorial feedback WICG/webcomponents#783

Closed

domenic mentioned this pull request Jan 27, 2019

Normative: GetExportedNames adjustments for late-defined export names #1306

Closed

dandclark added a commit to dandclark/webcomponents that referenced this pull request Feb 9, 2019

Rebasing ES spec changes on tc39/ecma262#1311. Misc other fixes/refac…

bf7e522

…torings.

ljharb added the es2019 label Feb 10, 2019

dandclark mentioned this pull request Feb 13, 2019

Update HTML Modules Proposed Spec changes and initial proposal document WICG/webcomponents#793

Merged

bterlson merged commit b012019 into tc39:master Feb 26, 2019

jmdyck mentioned this pull request Feb 27, 2019

Misc Editorial #1456

Merged

littledan mentioned this pull request Mar 2, 2019

Rebase and clarify text WebAssembly/esm-integration#22

Merged

littledan mentioned this pull request Mar 19, 2019

[[Status]] field not necessarily defined in GetModuleNamespace #1455

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layering: Enable cyclic dependencies with non-STMR module types #1311

Layering: Enable cyclic dependencies with non-STMR module types #1311

linclark commented Sep 24, 2018

jmdyck Sep 24, 2018

jmdyck Sep 24, 2018

jmdyck Sep 24, 2018

jmdyck Sep 24, 2018

jmdyck Sep 24, 2018

jmdyck Sep 24, 2018

littledan commented Oct 12, 2018

littledan commented Oct 19, 2018

littledan commented Oct 27, 2018

zenparsing Dec 13, 2018

domenic commented Jan 17, 2019

ljharb commented Jan 17, 2019

littledan commented Feb 16, 2019

zenparsing commented Feb 16, 2019

MylesBorins commented Feb 26, 2019

bterlson commented Feb 26, 2019

littledan commented Feb 28, 2019

Layering: Enable cyclic dependencies with non-STMR module types #1311

Layering: Enable cyclic dependencies with non-STMR module types #1311

Conversation

linclark commented Sep 24, 2018

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

jmdyck Sep 24, 2018

Choose a reason for hiding this comment

littledan commented Oct 12, 2018

littledan commented Oct 19, 2018

littledan commented Oct 27, 2018

zenparsing Dec 13, 2018

Choose a reason for hiding this comment

domenic commented Jan 17, 2019

ljharb commented Jan 17, 2019

littledan commented Feb 16, 2019

zenparsing commented Feb 16, 2019

MylesBorins commented Feb 26, 2019

bterlson commented Feb 26, 2019

littledan commented Feb 28, 2019