Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Dynamic Modules Status #188

Closed
guybedford opened this issue Sep 28, 2018 · 17 comments
Closed

Dynamic Modules Status #188

guybedford opened this issue Sep 28, 2018 · 17 comments

Comments

@guybedford
Copy link
Contributor

guybedford commented Sep 28, 2018

Update 5/11/2018: The Dynamic Modules implementation is now complete in v8, pending further review (at https://chromium-review.googlesource.com/c/v8/v8/+/1303725). This means we can have named exports in transparent interop of ESM from CJS

As you know, I've been pushing a Dynamic Modules specification for ensuring named exports on CommonJS while retaining a correct execution order in transparent initerop. This spec has been carefully worked out at https://github.com/guybedford/proposal-dynamic-modules.

While transparent interop remains a possibility, I believe this specification and work is important to follow so we can provide the best experience to users.

Initially this was created as a Stage 1 TC39 proposal, but it turns out the model is better served by having a Node-specific module record specification implemented in V8, just like Web Assembly will be its own module specification outside of TC39.

This way only a few ECMA262 specification changes are needed to support dynamic modules - tc39/ecma262#1306.

This PR was discussed at the latest TC39 meeting, and it was determined that it should only land following implementation work / feedback.

Thus, the next steps on Dynamic Modules here are:

  1. Setup the Dynamic Modules spec as a Node.js specification moving it from my personal account to the Node.js repo account. I've requested this here - transfer of dynamic-modules-proposal repo into nodejs organization admin#231
  2. Work with v8 to get an implementation together. I will be blocking off a full week or two as needed here to do the C++ work myself in the coming month, if anyone else is able to assist with this please let me know - it would be great to pair on the work.
  3. With implementation feedback from v8, get the spec changes merged back into ECMA262.

While this work all rests on a hypothetical, I hope we can all appreciate it is an important consideration on that hypothetical, and should be pushed regardless.

@devsnek
Copy link
Member

devsnek commented Sep 28, 2018

i'd be happy to help with the v8 impl.

@jkrems
Copy link
Contributor

jkrems commented Oct 3, 2018

I've done some work on import.meta in V8 in the past. Happy to help!

@guybedford
Copy link
Contributor Author

Thanks @jkrems, whats the best way to reach you to discuss the details further?

@jkrems
Copy link
Contributor

jkrems commented Oct 3, 2018

Most likely email or GChat (jan.krems@groupon.com) or Twitter DM (@jankrems). The former definitely is the more reliable way of reaching me quickly.

@guybedford
Copy link
Contributor Author

@jkrems ok I believe I've sent you an invite there, but let me know if you don't see it.

@jkrems
Copy link
Contributor

jkrems commented Oct 3, 2018

I'm not seeing an invite (including in my spam). Just in case you can also try my personal email (jan.krems@gmail.com).

@jkrems
Copy link
Contributor

jkrems commented Oct 3, 2018

Let me know if there's any communication method you'd prefer in case this doesn't work out. :)

@guybedford
Copy link
Contributor Author

guybedford commented Oct 15, 2018

To further update on the spec and implementation here:

The API can be seen in the test case here using HostExecuteDynamicModuleCallback and module->SetExport.

The following implementation steps are pending:

  • Handle namespace creation for dynamic modules
  • Provide and test linking error for undefined exports on dynamic modules
  • Provide and test linking error in uninstantiated circular leaf edge case
  • Further review on the implementation towards landing in v8

@jkrems @devsnek thanks both of you for feedback so far, do try it out again if you can!

@jkrems
Copy link
Contributor

jkrems commented Oct 15, 2018

Nice work! I played around with the test case to try some more module graphs (e.g. multiple imports of the same dynamic import) and it worked without a hitch. :)

(There's a single else on its own line in api.cc but I think the V8 code formatter should be able to take care of that..?)

@devsnek
Copy link
Member

devsnek commented Oct 15, 2018

@guybedford nice work! i have some feedback on the v8 internals that i can put up once you have a CL.

@guybedford
Copy link
Contributor Author

@jkrems awesome to hear it didn't immediately break :)

@devsnek that would be great - feel free to leave feedback at the repo there in the mean time, as it will probably take me a few more days yet to get to items 1 - 3.

@guybedford
Copy link
Contributor Author

Further update on dynamic modules - the implementation at guybedford/v8@master...guybedford:dynamic-modules is now fully spec-complete, and just needs some refactoring work on the data structure and module class side.

@devsnek you mentioned you had some ideas for how to handle the script metadata part of dynamic modules - would value your thoughts on that as I'm starting work on that now.

@guybedford
Copy link
Contributor Author

PR for review at guybedford/v8#2. Comments welcome.

@guybedford guybedford changed the title Update on Dynamic Modules Dynamic Modules Status Nov 5, 2018
@guybedford
Copy link
Contributor Author

The implementation of dynamic modules was finally completed to spec (and the spec worked out as written). Review at https://chromium-review.googlesource.com/c/v8/v8/+/1303725.

@guybedford guybedford removed the modules-agenda To be discussed in a meeting label Nov 8, 2018
@MylesBorins MylesBorins added the modules-agenda To be discussed in a meeting label Dec 4, 2018
@MylesBorins
Copy link
Contributor

I'd like us to discuss this one more time in tomorrow's meeting and ensure we have explicit consensus from our group that we want this moving forward.

@guybedford
Copy link
Contributor Author

Sure, I'd be happy to provide an update here on how last week's TC39 went.

@guybedford
Copy link
Contributor Author

As per the meeting, I've posted a Node.js issue with a status update at nodejs/node#24894.

@GeoffreyBooth GeoffreyBooth removed modules-agenda To be discussed in a meeting labels May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants