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

Worker: Disable relative import specifiers #5216

Closed
nayeemrmn opened this issue May 10, 2020 · 4 comments · Fixed by #5266
Closed

Worker: Disable relative import specifiers #5216

nayeemrmn opened this issue May 10, 2020 · 4 comments · Fixed by #5266
Labels
cli related to cli/ dir web related to Web APIs

Comments

@nayeemrmn
Copy link
Collaborator

A quick check on Chrome shows that relative import specifiers for Workers are resolved from globalThis.location. We don't have this concept at the moment so it should fail.

(Currently our Worker resolves from the main module, which used to be our globalThis.location.)

@kitsonk
Copy link
Contributor

kitsonk commented May 10, 2020

Hmmm... module resolution logic will always differ from a browser and a runtime. self.location would seem to be easier to solve than window.location too. I think it is best to agree to the semantics instead of just ripping it out.

@nayeemrmn
Copy link
Collaborator Author

Good point, we can keep it enabled for Workers spawned from within Workers since we do have a clear self.location (URL of the Worker script). But window.location is a huge thing which ties into a bunch of other issues... we should just disable it for now IMO.

new Worker(new URL("./worker.ts", import.meta.url), { type: "module" });

is going to be the common usage anyway.

@bartlomieju bartlomieju added cli related to cli/ dir public API related to "Deno" namespace in JS web related to Web APIs and removed public API related to "Deno" namespace in JS labels May 21, 2020
@aniketbiprojit
Copy link

Can someone explain why this seems a good functionality as opposed to relative path? Is it because of the complexity to implement it or just opinion based?

@nayeemrmn
Copy link
Collaborator Author

@aniketbiprojit It's about being analogous to the web. We don't have a globalThis.location yet so anything that's supposed to be relative to it should fail for now. Ref #4981.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir web related to Web APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants