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

importHook / importNowHook timings #20

Open
guybedford opened this issue Oct 21, 2020 · 1 comment
Open

importHook / importNowHook timings #20

guybedford opened this issue Oct 21, 2020 · 1 comment

Comments

@guybedford
Copy link
Collaborator

It was discussed today sharing the importHook and importNowHook while still having a Promise returned from importHook.

It was my understanding that the original driving use case for the importNowHook was cases where the module binding is needed to be accessed synchronously in the same function for eg loading builtins or any precompiled modules that are already available for execution.

Therefore, if importHook were to replicate this behaviour, this seems to imply that importHook would want to not be doing any task queue runs at all if there is no top-level await in the graph. The result of this is that execution errors would become direct exceptions of the importHook call instead of promise rejections returned from it. This does seem a little concerning if it is the case.

I'm still getting to grips with the exact semantics here, so I was just looking to verify if my understanding of the above matches what is being considered?

@kriskowal
Copy link
Member

kriskowal commented Oct 21, 2020

There were two relevant topics discussed this morning.

Let’s assume we rename importHook to loadHook and importNowHook to loadNowHook to more properly reflect their roles. These are hooks that return a module record for some module that has been loaded. loadHook is async and loadNowHook is sync. Modules that are synchronously available in a compartment from birth enter through the moduleMap, not through the loader hook(s).

The topics we discussed were:

  1. Removing the loadNowHook compartment option because loading is inherently asynchronous. Any synchronous need is better served by moduleMap injection. (The Moddable/XS folks may have evidence to contradict these assertions!) cc @phoddie @patrick-soquet
  2. Possibly removing compartment.importNow because it composes poorly with top-level-await. It is possible for one program to call compartment.importNow(x) when neither x nor any of its transitive dependencies use top-level-await. If any of those transitive dependencies introduce top-level-await, the compartment would be obliged to throw an error here, or return a module namespace that has not been fully initialized. The alternative is to to keep compartment.importNow() and socialize the hazard so folks know that adding the first top-level-await to any module is a breaking change and would oblige the module author to bump their major version.

Your argument applies to the former, though the issue came up on the slide discussing the latter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants