{"action":"opened","issue":{"url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/5875","repository_url":"https://api.github.com/repos/Microsoft/ChakraCore","labels_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/5875/labels{/name}","comments_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/5875/comments","events_url":"https://api.github.com/repos/Microsoft/ChakraCore/issues/5875/events","html_url":"https://github.com/Microsoft/ChakraCore/issues/5875","id":390892290,"node_id":"MDU6SXNzdWUzOTA4OTIyOTA=","number":5875,"title":"Suggestion: Module API divide JsSetModuleHostInfo","user":{"login":"rhuanjl","id":30090552,"node_id":"MDQ6VXNlcjMwMDkwNTUy","avatar_url":"https://avatars3.githubusercontent.com/u/30090552?v=4","gravatar_id":"","url":"https://api.github.com/users/rhuanjl","html_url":"https://github.com/rhuanjl","followers_url":"https://api.github.com/users/rhuanjl/followers","following_url":"https://api.github.com/users/rhuanjl/following{/other_user}","gists_url":"https://api.github.com/users/rhuanjl/gists{/gist_id}","starred_url":"https://api.github.com/users/rhuanjl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rhuanjl/subscriptions","organizations_url":"https://api.github.com/users/rhuanjl/orgs","repos_url":"https://api.github.com/users/rhuanjl/repos","events_url":"https://api.github.com/users/rhuanjl/events{/privacy}","received_events_url":"https://api.github.com/users/rhuanjl/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-12-13T22:41:31Z","updated_at":"2018-12-13T22:41:31Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"```cc\r\nJsSetModuleHostInfo(\r\n    _In_ JsModuleRecord requestModule,\r\n    _In_ JsModuleHostInfoKind moduleHostInfo,\r\n    _In_ void* hostInfo)\r\n```\r\n\r\n**Currently JsSetModuleHostInfo is used for two fundamentally different types of thing**\r\n\r\n1. It is used to set properties relating to an individual module, when setting any of these it is set onto the module record you pass in - these can be:\r\n    i. an URL\r\n    ii. host defined info\r\n    iii. an exception\r\n1. It is also used to setup the callbacks for module loading - these are set onto the script context (though the api uses GetScriptContext from the module record to know which context) these are:\r\n    i. FetchImportedModuleCallback (used for child modules AND dynamic import from a module)\r\n    ii. FetchImportedModuleFromScriptCallback (used for dynamic import from a script)\r\n    iii. NotifyModuleReadyCallback (used to say a module is ready to execute)\r\n\r\n**There are a few issues with this:**\r\n1. It creates a potential for confusion - e.g. as the callbacks you set are actually set on the scriptContext but you're specifying a moduleRecord to set them on a user may think they need to set them each time they make a moduleRecord (which they don't) AND worse a user may think they can use a different callback for a different module in the same context - the latter one will override the first one and produce unexpected behaviour.\r\n\r\n1. If an embedder wishes to setup dynamic import from scripts they have to create a throwaway module record to enable them to set the callbacks in advance.\r\n\r\n1. If an embedder wishes to support only static import they either need a throwaway module record to set the callbacks in advance or they need to set them on their main entry point module OR they may (as ch actually does) unnecessarily set them every time a module record is created.\r\n\r\n**Suggestion:**\r\n1. Leave this API functioning as is so as not to break any existing implementations that use it.\r\n1. Add a new additional API for setting the callbacks that takes ScriptContext and callback parameters and not a moduleRecord\r\n1. Update documentation to reccomend the use of this new api for setting callbacks and suggest JsSetModuleHostInfo just for the module specific info (URL, HostDefinedInfo and loading Exceptions)\r\n\r\n**Note:**\r\nAs usual with any suggestion I make a PR to implement the suggestion is on offer."}}
{"actor":{"display_login":"rhuanjl"}}