Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modules: Coordinating ESM implementation in Core #8866

Closed
jasnell opened this issue Sep 30, 2016 · 19 comments
Closed

Modules: Coordinating ESM implementation in Core #8866

jasnell opened this issue Sep 30, 2016 · 19 comments
Labels
discuss Issues opened for discussions and feedbacks. meta Issues and PRs related to the general management of the project. module Issues and PRs related to the module subsystem.

Comments

@jasnell
Copy link
Member

jasnell commented Sep 30, 2016

@nodejs/tc39 @nodejs/v8 @nodejs/node-chakracore ... following the TC-39 meeting this week, there is a proposed path forward for getting modules implemented in core. While the node-eps documents need to be updated, it would be good to begin coordinating the various threads that would need to be pulled together for working out an actual implementation.

@nodejs/v8 and @nodejs/node-chakracore teams... can you please respond to this thread with details or links to each VM's plan and timeline regarding ECMAScript Module implementation. Pointers to existing documentation is perfectly fine, I just want to use this issue as a coordination point.

@jasnell jasnell added module Issues and PRs related to the module subsystem. discuss Issues opened for discussions and feedbacks. labels Sep 30, 2016
@targos
Copy link
Member

targos commented Sep 30, 2016

This is the V8 bug tracking Module implementation: https://bugs.chromium.org/p/v8/issues/detail?id=1569

@ofrobots
Copy link
Contributor

/cc @ajklein for V8 plans

@Fishrock123
Copy link
Contributor

ChakraCore Appears to have 100% implemented but documentation doesn't seem to be available yet.
I have yet to be able to make more progress attempting to shim v8module.

@chrisdickinson has a branch that attempts to use the current V8 API on master, which is only somewhat functional on the V8 end but completely functional on ours; there is a full implementation of a html-spec compliant JS loader included as well: https://github.com/chrisdickinson/node-1/tree/implement-esm

@Fishrock123 Fishrock123 added the meta Issues and PRs related to the general management of the project. label Sep 30, 2016
@mikeal
Copy link
Contributor

mikeal commented Sep 30, 2016

FYI, at the TC39 meeting we were told the html-spec for the loader is "in flux" and is likely to change quite dramatically. In fact, we expect it to change quite a bit based on our feedback.

@Fishrock123
Copy link
Contributor

Ok, we'd might as well still work backwards from an impl that has anything that may not change already in place though, imo. Idk, we'll have to see what those changes are like but I think the spec as it exists today will be a good enough starting point to prototype with.

@ajklein
Copy link
Contributor

ajklein commented Oct 3, 2016

V8 is quite close to a complete implementation of what's in ES2015 (note that it's only implemented in our interpreter, so passing --ignition is required to make it work at the moment). Our API probably isn't sufficient for Node's use yet: it assumes all modules will be created as "Source Text Module Records" (in spec-speak), since that's what we expect to need for an implementation of <script type="module"> in Blink. It would be useful to understand Node's API requirements.

Regarding the design explorations at TC39, we're still early enough that nothing from that conversation is likely to land in V8 as part of our initial modules implementation. So it would be great if we could find a sufficient API surface that experimentation in that area could happen outside of V8.

@domenic
Copy link
Contributor

domenic commented Oct 4, 2016

FYI, at the TC39 meeting we were told the html-spec for the loader is "in flux" and is likely to change quite dramatically. In fact, we expect it to change quite a bit based on our feedback.

I think something was lost in translation there. What's in flux is https://whatwg.github.io/loader/ (a "collection of interesting ideas" about loading in various environments), not https://html.spec.whatwg.org/multipage/webappapis.html#fetching-scripts and related sections (the HTML spec loader). The browser loader is finalized, pending implementation feedback (although implementations have progressed quite far so I don't anticipate much more).

I would definitely not bother implementing the collection of interesting ideas, as most of it is going to be removed and changed in various ways.

@bmeck
Copy link
Member

bmeck commented Oct 4, 2016

@ajklein I think the solutions we described that VMs will need at the meeting are as follows:

  1. Late linking validation : Need a new Module Record type that's shape is not validated until dependencies are run through ModuleEvaluation.
    • This requires a spec change to loosen ModuleDeclarationInstanciation on Source Text Module Records and implement a guard in ModuleEvaluation.
  2. Property Delegation : New Module Record should be able to delegate to a value's properties rather than a scope's bindings. It was pointed out that this has similar effects as global variables wrt side-effects.
    • Does not require a spec change to existing structures, though requires some addition of a concrete Module Record type somewhere (outside of Ecma262?).
  3. Re-linking : Currently linking is permanent, however until ModuleEvaluation occurs, some modules like meow may end up rewriting their parent's behavior to delegate to a new dynamically created module instead of the existing dep. Having linking become permanent after evaluation.
    • This requires a spec change to loosen ModuleDeclarationInstanciation on Source Text Module Records.

We have not discussed the API design for these or some other things discussed like meta-data to declare the shape of a late linked module up front. Would it make sense to give me time to write up a spec for the new type of ModuleRecord / gather the tweaks to spec prior to jumping into a C++ API design discussion?

@ajklein
Copy link
Contributor

ajklein commented Oct 4, 2016

@bmeck When I talk about "API requirements", I mean something higher-level than actual C++ code, say, "ability to create a Module Record from outside the API and pass it into V8 to be linked".

But it does seem like we may still be at too early a stage for that: your point 2, for example, sounds like big extension to what we discussed at the meeting.

@bmeck
Copy link
Member

bmeck commented Oct 4, 2016

@ajklein if the values go out of sync for "plucked" variables, it would make more sense just to not support named imports from CJS IMO (though others differ on this); Not having live delegation/named imports changes many parts of this discussion.

@joshgav
Copy link
Contributor

joshgav commented Oct 5, 2016

/cc @digitalinfinity @boingoing @kunalspathak re Chakra

@boingoing
Copy link
Contributor

boingoing commented Oct 6, 2016

@joshgav Chakra has a compliant implementation of ECMAScript modules now (barring any bugs). We don't have support for the Loader or BrowserLoader objects though we made some progress on the whatwg spec (chakra-core/ChakraCore#1472) which has not been pulled-in to the ChakraCore repo, yet. Looks like we will need to refactor this after the dust settles on a converged design.

Since the only way to test ESM is with help from the host (ie: static modules via <script>), testing with ChakraCore is possible via ch.exe with "-es6module" test flag. The feature can also be enabled in Edge on Windows Insider builds via the "Enable experimental JavaScript features" toggle under about:flags. I believe the current Insider flight (14936) has a mostly-complete modules implementation (missing module namespace object and has some bugs IIRC) but the next Windows Insider build should have our complete ESM implementation.

@Fishrock123
Copy link
Contributor

@boingoing is there docs on how we can use those experimental features in chakracore to power a node prototype of ES Modules?

@joshgav
Copy link
Contributor

joshgav commented Dec 8, 2016

@Fishrock123 @chrisdickinson for Source Text Module Records (from the 2015 spec) is the main ask from Chakra to provide a shim that matches V8's CompileModule API?

To illustrate, the way to provide that would likely be to add an experimental JsParseModule API to JSRT and then shim that.

/cc @liminzhu

@Fishrock123
Copy link
Contributor

@joshgav Yes and no, we could shim it (help would be appreciated) but there isn't an expectation to necessarily have any change in Chakra itself for this at the current time.

To illustrate, the way to provide that would likely be to add an experimental JsParseModule API to JSRT and then shim that.

Is that not already the case? It must be exposed already somehow, tests and such use it IIRC.

@boingoing
Copy link
Contributor

boingoing commented Jan 6, 2017

@Fishrock123 Here's an outline of how the Jsrt API for ES modules works and how we use it in our test harness.

Jsrt provides a scaffolding of sorts to support loading ES modules. The Jsrt API we need to work with are located in ChakraCore.h:

  • CHAKRA_API JsInitializeModuleRecord(_In_opt_ JsModuleRecord referencingModule, _In_ JsValueRef normalizedSpecifier, _Outptr_result_maybenull_ JsModuleRecord* moduleRecord);
  • CHAKRA_API JsParseModuleSource(_In_ JsModuleRecord requestModule, _In_ JsSourceContext sourceContext, _In_ BYTE* script, _In_ unsigned int scriptLength, _In_ JsParseModuleSourceFlags sourceFlag, _Outptr_result_maybenull_ JsValueRef* exceptionValueRef);
  • CHAKRA_API JsModuleEvaluation(_In_ JsModuleRecord requestModule, _Outptr_result_maybenull_ JsValueRef* result);
  • CHAKRA_API JsSetModuleHostInfo(_In_ JsModuleRecord requestModule, _In_ JsModuleHostInfoKind moduleHostInfo, _In_ void* hostInfo);
  • CHAKRA_API JsGetModuleHostInfo(_In_ JsModuleRecord requestModule, _In_ JsModuleHostInfoKind moduleHostInfo, _Outptr_result_maybenull_ void** hostInfo);

We need the ChakraCore host to glue these pieces together and provide a little boiler-plate code in order to get full support for loading and running module code. We have a reference implementation in our ChakraCore test host (ch.exe) which is implemented largely in WScriptJsrt.cpp (https://github.com/Microsoft/ChakraCore/blob/master/bin/ch/WScriptJsrt.cpp).

Use JsSetModuleHostInfo to set a callback for FetchImportedModule. This function will be called-back from JsParseModuleSource when the parser comes across a dependent module (in an import clause, for example). Our implementation for ch.exe is in (WScriptJsrt::FetchImportedModule). Note that the module needs to be actually fetched asynchronously. In WScriptJsrt.cpp, we use a message queue to accomplish this. See WScriptJsrt::ModuleMessage. The signature of the callback function is:
typedef JsErrorCode(CHAKRA_CALLBACK * FetchImportedModuleCallBack)(_In_ JsModuleRecord referencingModule, _In_ JsValueRef specifier, _Outptr_result_maybenull_ JsModuleRecord* dependentModuleRecord);

Use JsSetModuleHostInfo to set a callback for NotifyModuleReady. This function will be called-back after the module is instantiated and ready to be executed (ModuleDeclarationInstantiation). Host needs to call back to execute the module via JsModuleEvaluation but this needs to also be done asynchronously. We accomplish this in the same way as FetchImportedModule via WScriptJsrt::ModuleMessage. The signature of the callback function is:
typedef JsErrorCode(CHAKRA_CALLBACK * NotifyModuleReadyCallback)(_In_opt_ JsModuleRecord referencingModule, _In_opt_ JsValueRef exceptionVar);

Keep a mapping from module specifiers to module record entries. If we request to fetch a module we already know about, the same module record should be used. We use this in our parse method and fetch method, basically when we need to look up a module record based on a module specifier. This map is implemented by WScriptJsrt::moduleRecordMap in ch.exe.

Load a root module. Create a new module record if we need one and use JsInitializeModuleRecord to initialize it. Pass nullptr as the referencingModule argument - this is the parent module, which is null for a root module. Also, we need to call JsSetModuleHostInfo to indicate the module record is host-defined (like, JsSetModuleHostInfo(moduleRecord, JsModuleHostInfo_HostDefined, specifier)). After setting-up a module record, parse the source text via JsParseModuleSource. This call is synchronous and will call the FetchImportedModule callback (which was set via JsSetModuleHostInfo) for all children module specifiers. Check out WScriptJsrt::LoadModuleFromString for an idea of how this works.

@joshgav
Copy link
Contributor

joshgav commented Jan 12, 2017

@bmeck gave an update in yesterday's CTC meeting, notes in CTC#60 for the moment.

@Trott
Copy link
Member

Trott commented Jul 15, 2017

Is it useful for this to stay open?

@bmeck
Copy link
Member

bmeck commented Jul 15, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. meta Issues and PRs related to the general management of the project. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

12 participants