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

Module worker descendant script fetching regression #9513

Closed
domenic opened this issue Jul 13, 2023 · 4 comments · Fixed by #9520
Closed

Module worker descendant script fetching regression #9513

domenic opened this issue Jul 13, 2023 · 4 comments · Fixed by #9520

Comments

@domenic
Copy link
Member

domenic commented Jul 13, 2023

Discussion continued from #9499.

Apparently in #8253 we stopped using the fetch client settings object (i.e., the "outer" environment settings object) when fetching the descendants of a module worker. This is incorrect.

You can find the old spec in https://html.spec.whatwg.org/commit-snapshots/2edc8089d4aa2db44a324f44b3fe4e4898524217/#fetch-a-worklet/module-worker-script-graph

This impacts, e.g., what CSP or other policies applies to the module fetches. So this is definitely a bug.

@nicolo-ribaudo, can you work on fixing this to the state before your PR?

@nicolo-ribaudo
Copy link
Contributor

Yes I can work on this 👍

@allstarschh
Copy link

@domenic Can you explain why fetching the descendants needs the environment settings object from the document? Because after fetching the top-level worker module script, worker will have its own environment settings object.

@domenic
Copy link
Member Author

domenic commented Jul 13, 2023

Because after fetching the top-level worker module script, worker will have its own environment settings object.

That's not true, or at least if it is, it is only because of weird specification-only things. You can't create the worker thread until you have all the code you plan to run inside of it, which means the whole dependency graph.

@domfarolino
Copy link
Member

Dynamic imports inside of module works will use the worker script's settings object though, right?

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

Successfully merging a pull request may close this issue.

4 participants