-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
fix(worker): worker import.meta.url should not depends on document in iife mode #12629
Conversation
Run & review this pull request in StackBlitz Codeflow. |
document
…There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
resolveImportMeta(property, { chunkId, format }) { | ||
// document is undefined in the worker, so we need to avoid it in iife | ||
if (property === 'url' && format === 'iife') { | ||
return `new URL('${chunkId}', self.location.href).href` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return `new URL('${chunkId}', self.location.href).href` | |
return 'self.location.href' |
I just noticed that we should use self.location.href
here. location.href
returns the location of the worker and not the document.
https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope/location
new URL('${chunkId}', self.location.href).href
is be like http://localhost:4173/assets/assets/worker.js
which is not correct, because chunkId
is like /assets/worker.js
and self.location.href
is like http://localhost:4173/assets/worker.js
.
Will this go in 4.2.2 ? |
Seems like it didn't make it. I still get… var de = document.currentScript && document.currentScript.src || new URL("assets/worker-b4ac415f.js",document.baseURI).href; …in my code. It works fine in |
@tomayac you can try it out on vite@4.3.0-beta.8. If no regressions are reported, it will be tagged as 4.3 stable tomorrow. |
Oh, amazing! Confirming that it works fine on |
I'm still getting this issue on this line here export const packageInfo = { name: '@polkadot/x-textdecoder', path: new URL(import.meta.url).pathname, type: 'esm', version: '11.1.3' }; still get compiled to const yw = {
name: "@polkadot/x-textdecoder",
path: {
url: document.currentScript && document.currentScript.src || new URL("assets/worker-4a3b769e.js",document.baseURI).href
} && self.location.href ? new URL(self.location.href).pathname.substring(0, new URL(self.location.href).pathname.lastIndexOf("/") + 1) : "auto",
type: "esm",
version: "11.1.3"
} |
@tien please create a new issue with a minimal reproduction against the latest Vite version (and you can link this PR for reference). A comment on a merged PR doesn't allow us to track your problem properly |
Description
fix #12611
import.meta.url
will be transformed to something likedocument.currentScript&&document.currentScript.src||new URL("a.js",document.baseURI).href
in iife mode. Butdocument
is undefined in the worker context.This PR directly transforms it to
new URL('a.js', self.location.href).href
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).