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

Initial AsyncContext.Snapshot #93

Open
littledan opened this issue Jun 10, 2024 · 4 comments · May be fixed by #98
Open

Initial AsyncContext.Snapshot #93

littledan opened this issue Jun 10, 2024 · 4 comments · May be fixed by #98

Comments

@littledan
Copy link
Member

Across the JS spec and web integration PR, I don't see where the initial AsyncContext.Snapshot is set up. IMO this context should be created by the host, so that they initialize certain AsyncContext.Variables if they want. We should also make sure that each module is run in the context of that initial snapshot, both to reduce Zalgo (imagine racing dynamic imports from different contexts of the same module) and to ensure the functioning of an the idiom to use a "null snapshot" by .run-ing a snapshot taken at the top level of the module.

@mhofman
Copy link
Member

mhofman commented Jun 10, 2024

make sure that each module is run in the context of that initial snapshot
dynamic imports from different contexts of the same module

I actually have the opposite concern, that dynamic import, a language feature, would allow the user code to "escape" any async context setup by an environment that loads user code.

reduce Zalgo (imagine racing dynamic imports

Is there nothing the spec says here on the order in which dynamic imports of the same module should resolve? I agree it's a weird action at a distance to be able for a module to notice where it was imported from, but that seems to be a bridge we're already crossing with deferred module evaluation and some host providing async stack traces. Actually we should decide what the context should be for a deferred module evaluation (the one of the import or the one of the first access which triggered evaluation?).

I don't see where the initial AsyncContext.Snapshot is set up. IMO this context should be created by the host, so that they initialize certain AsyncContext.Variables if they want.

At first I thought they could do that when initializing the realm. But then I realized that some things might need to be setup when initializing the agent. For example I don't think a new ShadowRealm has any context of its own at creation, but the evaluations and imports should likely happen in the context of the caller.

@littledan
Copy link
Member Author

Dynamic import already allows “escaping” to access the original global variable and such. Maybe compartments should include a particular default outer context snapshot.

You make a good argument that import defer should also restore the original context snapshot.

@shicks
Copy link
Contributor

shicks commented Jun 26, 2024

IIUC, ordinary top-level imports will always occur in the initial snapshot, right? So the only way to ever get the top level of a module running in a user-written snapshot would be with a dynamic or deferred import (or a transitive top-level import therein) if the spec were to allow it.

I find the lack of consistency to be a little strange, but maybe there's a use case for being able to set the context a module runs in? It seems to me that even if dynamic imports went one way, deferred imports could still reasonably take either approach. So the options are

  1. all module scopes always run in the initial snapshot
  2. dynamic imports run in (probably) the snapshot of the first call that imported it, while deferred imports run in the initial snapshot
  3. dynamic and deferred imports both run in the snapshot of the first call/use that triggered the import

It's also worth considering script execution snapshot - what should v.run(1, () => document.write('<script>console.log(v.get())</script>')); log? I think here the spirit of "flow-around/registration context" would say undefined, while the "causal context" approach could reasonably log 1 as the most relevant cause.

@littledan
Copy link
Member Author

I think dynamic import should also run in the top-level snapshot, as should import defer.

@legendecas legendecas linked a pull request Jul 15, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants