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

linclark · 2018-09-24T01:50:02Z

The terminology between WebAssembly and ES modules differs when it comes to instantiation, which causes some confusion for the integration of the two systems. Ideally, we wouldn't use the same term to mean two different things between the specs.

After talking about it with multiple people, it seems like calling it "the linking phase" might be a better name for this phase anyway. Developers are often confused about what actually happens during instantiation and why it's separated from evaluation. I believe "linking" more clearly communicates the justification for this being its own phase.

Note: This depends on #1311. The relevant commit for this PR is the last one. Once #1311 is merged, I can rebase this to remove those commits.

devsnek

I can't speak for the spec changes themselves but the name change is great 👍

annevk · 2018-09-24T09:19:43Z

If this change is made, it would also require downstream changes, e.g., to https://html.spec.whatwg.org/.

cc @domenic

allenwb · 2018-09-24T17:13:43Z

I also like this name change.

dherman · 2018-09-27T16:44:23Z

I've always been a fan of Legend of Zelda. 👍

spec.html

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


littledan · 2018-10-18T10:20:16Z

The name change seems good to me; I'm happy to see the positive reaction here. Let's discuss this in the November 2018 TC39 meeting.

domenic · 2018-10-18T10:24:10Z

I was hoping we could land layering changes outside of plenary?

littledan · 2018-10-18T12:00:12Z

Oh, sure, I'd be fine with landing this outside of plenary. Let's just not delay this longer than that.

Editors, what do you think?

bterlson · 2018-10-24T19:13:34Z

We're fine with this change; will finish reviewing #1311 and will pull these in together.

ljharb

The diff for just the rename can be viewed here: linclark/ecma262@cyclic-mr-2...linclark:rename-instantiate

littledan · 2019-03-02T09:01:37Z

Ping on this change; I was hoping it would be pulled in together with #1311 per #1312 (comment) .

ljharb · 2019-03-03T06:53:21Z

@littledan this one won't be in 2019, and it'll need to be rebased as well (that will also help it be more easily reviewed, since it wasn't set up as a stacked PR)

littledan · 2019-03-03T08:15:38Z

Rebase out for review at #1465 . Please let me know if you need anything else.

ljharb · 2019-03-03T16:16:34Z

To clarify, please do not open redundant PRs; either the PR author has to rebase it, or you can post a link to your branch, and this PR can updated by an editor with those commits.

littledan · 2019-03-04T08:52:13Z

Oops, didn't realize I had write access to this branch. Rebased here.

zenparsing

Changes look good, and in general I like the name change. I'm curious about the churn that it might create for JS engine APIs, though.

Thoughts @ajklein? Other implementers?

spec.html

ajklein · 2019-03-04T20:00:52Z

I like the name change conceptually, so happy to see this move forward. I don't think that we'd change V8's C++ naming just because of this spec change; rather we'd use the new name if we had other reasons that the API needed changing. @GeorgNeis in case he agrees or disagrees

littledan · 2019-03-15T22:50:58Z

Do you need anything else from me to review this patch? From #1312 (comment) , it sounds like implementation burden isn't a big issue here.

The terminology between WebAssembly and ES modules differs when it comes to instantiation, which causes some confusion for the integration of the two systems. Ideally, we wouldn't use the same term to mean two different things between the specs. After talking about it with multiple people, it seems like calling it "the linking phase" might be a better name for this phase anyway. Developers are often confused about what actually happens during instantiation and why it's separated from evaluation. I believe "linking" more clearly communicates the justification for this being its own phase.

Ms2ger · 2019-06-04T10:08:33Z

Is someone following up with other specs that currently use "Instantiate"? I'm thinking at least HTML, WebIDL, top-level-await and wasm/esm-integration.

littledan · 2019-06-25T06:38:54Z

@Ms2ger I haven't done this yet; if you are interested in fixing these other specs, it would be greatly appreciated.

See <tc39/ecma262#1312>.

* Update to current TLA draft. * Rename uninstantiated to unlinked. See <tc39/ecma262#1312>.

See <tc39/ecma262#1312>.

See tc39/ecma262#1312.

* Update to current TLA draft. * Rename uninstantiated to unlinked. See <tc39/ecma262#1312>.

devsnek approved these changes Sep 24, 2018

View reviewed changes

TimothyGu reviewed Oct 4, 2018

View reviewed changes

tschneidereit mentioned this pull request Oct 19, 2018

Early draft specification; modified explainer WebAssembly/esm-integration#21

Merged

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

ljharb requested review from a team and bmeck October 24, 2018 19:20

ljharb reviewed Nov 27, 2018

View reviewed changes

littledan added the has consensus This has committee consensus. label Feb 7, 2019

ljharb requested review from a team and zenparsing and removed request for bterlson February 28, 2019 19:56

littledan mentioned this pull request Mar 3, 2019

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

Closed

littledan force-pushed the rename-instantiate branch from c910453 to b7ba2f8 Compare March 4, 2019 08:51

zenparsing reviewed Mar 4, 2019

View reviewed changes

spec.html Outdated Show resolved Hide resolved

ljharb approved these changes Mar 16, 2019

View reviewed changes

ljharb removed the request for review from bmeck March 16, 2019 07:11

ljharb added the editorial change label Mar 16, 2019

ljharb force-pushed the rename-instantiate branch from 2cbf848 to 4f13bc9 Compare April 12, 2019 21:59

ljharb requested review from a team and zenparsing April 12, 2019 21:59

zenparsing approved these changes Jun 3, 2019

View reviewed changes

ljharb self-assigned this Jun 3, 2019

ljharb force-pushed the rename-instantiate branch from 4f13bc9 to 05c7620 Compare June 3, 2019 20:41

ljharb merged commit 05c7620 into tc39:master Jun 3, 2019

Ms2ger added a commit to whatwg/html that referenced this pull request Jun 25, 2019

Rename Module.Instantiate to Module.Link

04aee82

See <tc39/ecma262#1312>.

Ms2ger mentioned this pull request Jun 25, 2019

Rename Module.Instantiate to Module.Link whatwg/html#4733

Merged

Ms2ger added a commit to whatwg/webidl that referenced this pull request Jun 25, 2019

Rename Module.Instantiate to Module.Link

51559c7

See <tc39/ecma262#1312>.

This was referenced Jun 25, 2019

Rename Module.Instantiate to Module.Link whatwg/webidl#741

Merged

Editorial: Editor feedback tc39/proposal-top-level-await#107

Merged

Ms2ger added a commit to Ms2ger/esm-integration that referenced this pull request Jun 25, 2019

Rename uninstantiated to unlinked.

ec80376

See <tc39/ecma262#1312>.

littledan pushed a commit to WebAssembly/esm-integration that referenced this pull request Jun 25, 2019

Update to current TLA draft. (#31)

7bd5a38

* Update to current TLA draft. * Rename uninstantiated to unlinked. See <tc39/ecma262#1312>.

Ms2ger added a commit to whatwg/webidl that referenced this pull request Jun 26, 2019

Rename Module.Instantiate to Module.Link

c0d927a

See <tc39/ecma262#1312>.

domenic pushed a commit to whatwg/html that referenced this pull request Jul 2, 2019

Update for module records' Instantiate → Link rename

715d4ba

See tc39/ecma262#1312.

xtuc pushed a commit to xtuc/esm-integration that referenced this pull request Aug 13, 2019

Update to current TLA draft. (WebAssembly#31)

32eab6e

* Update to current TLA draft. * Rename uninstantiated to unlinked. See <tc39/ecma262#1312>.

jmdyck mentioned this pull request Aug 4, 2023

Rename "to link" to "to load" in the Source Text Module Records section #3141

Closed

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

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

Uh oh!

Conversation

linclark commented Sep 24, 2018

Uh oh!

devsnek left a comment

Choose a reason for hiding this comment

Uh oh!

annevk commented Sep 24, 2018

Uh oh!

allenwb commented Sep 24, 2018

Uh oh!

dherman commented Sep 27, 2018

Uh oh!

This comment was marked as resolved.

Uh oh!

littledan commented Oct 18, 2018

Uh oh!

domenic commented Oct 18, 2018

Uh oh!

littledan commented Oct 18, 2018

Uh oh!

bterlson commented Oct 24, 2018

Uh oh!

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

littledan commented Mar 2, 2019

Uh oh!

ljharb commented Mar 3, 2019

Uh oh!

littledan commented Mar 3, 2019

Uh oh!

ljharb commented Mar 3, 2019

Uh oh!

littledan commented Mar 4, 2019

Uh oh!

zenparsing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ajklein commented Mar 4, 2019

Uh oh!

littledan commented Mar 15, 2019

Uh oh!

Ms2ger commented Jun 4, 2019

Uh oh!

littledan commented Jun 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants