-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
JSRT module api #1254
JSRT module api #1254
Conversation
Add module support in chakracore by adding related JSRT APIs. This set of APIs are in chakracore.dll only, not in chakra.dll as we are using different binding for edge browser. The host is responsible for asynchronously fetch dependent modules and handle error conditions. The overall loading sequences: host chakacore.dll 1. Load top level script 2. ----> JsInitializeModuleRecord // for root module 3. ----> JsParseModuleSource // parse root module 4. FetchImportedModuleCallback <---- // optional for imported module 5. -----> JsInitializeModuleRecord // for nested module 6. load additional script 7. -----> JsParseModuleSource // for nested module 8. NotifyModuleReadyCallback <----- 9. -----> JsModuleEvaluation // for root module step 4 to step 7 could be happening in a loop, and step 5 should happen asynchronously like in a Promise. Step 9 will also evaluate all the nested modules. Port some existing test cases to ch. We need to support test262 hook in later checkin.
@boingoing can you take a look? |
JsParseModuleSource | ||
JsModuleEvaluation | ||
JsSetModuleHostInfo | ||
JsGetModuleHostInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should consider adding nativetests for these sets of api. nativetests should be the place for adding negative tests to validate the various inputs for these APIs which ch.exe is not supposed to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point; will leave out of this change though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this issue to track this #1269
/// </remarks> | ||
/// <param name="referencingModule">The referencingModule as in HostResolveImportedModule (15.2.1.17). nullptr if this is the top level module.</param> | ||
/// <param name="normalizedSpecifier">The host normalized specifier. This is the key to a unique ModuleRecord.</param> | ||
/// <param name="specifierLength">The length of normalizedSpecifier.</param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no parameter named specifierLength
#Resolved
Disable module tests in linux for now while I figure out why the import entries don't get resolved correctly in ch.
@@ -25,6 +25,9 @@ | |||
|
|||
#define ENABLE_TEST_HOOKS 1 | |||
#include "CommonDefines.h" | |||
#include <map> | |||
|
|||
#include <map> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like is included twice here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah merge resolution error. Will fix it with the Linux static build failure. #Resolved
@boingoing any additional feedback? |
LGTM |
Merge pull request #1254 from Yongqu:bugfix Add module support in chakracore by adding related JSRT APIs. This set of APIs are in chakracore.dll only, not in chakra.dll as we are using different binding for edge browser. The host is responsible for asynchronously fetch dependent modules and handle error conditions. The overall loading sequences: ``` host chakacore.dll 1. Load top level script 2. ----> JsInitializeModuleRecord // for root module 3. ----> JsParseModuleSource // parse root module 4. FetchImportedModuleCallback <---- // optional for imported module 5. -----> JsInitializeModuleRecord // for nested module 6. load additional script 7. -----> JsParseModuleSource // for nested module 8. NotifyModuleReadyCallback <----- 9. -----> JsModuleEvaluation // for root module ``` step 4 to step 7 could be happening in a loop, and step 5 should happen asynchronously like in a Promise. Step 9 will also evaluate all the nested modules. Port some existing test cases to ch. We need to support test262 hook in later checkin.
hi, |
@JohnMasen I’m afraid the message you have sent here is going to be lost. Can you please open an issue? |
Add module support in chakracore by adding related JSRT APIs. This set of
APIs are in chakracore.dll only, not in chakra.dll as we are using different binding
for edge browser. The host is responsible for asynchronously fetch dependent modules
and handle error conditions.
The overall loading sequences:
step 4 to step 7 could be happening in a loop, and step 5 should happen asynchronously
like in a Promise. Step 9 will also evaluate all the nested modules.
Port some existing test cases to ch. We need to support test262 hook in later checkin.