-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
feat: use import.meta.url
instead of self.location
#14377
Conversation
Run & review this pull request in StackBlitz Codeflow. |
`new URL(${JSON.stringify(builtUrl)}, self.location)`, | ||
// add `'' +` to skip vite:asset-import-meta-url plugin | ||
`new URL('' + ${JSON.stringify(builtUrl)}, import.meta.url)`, |
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.
/foo
gets replaced with /@fs/foo
by vite:asset-import-meta-url plugin. Maybe we should add /* @vite-ignore */
for new URL
as well?
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.
Using /* @vite-ignore */
works for me 👍
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.
I'll make a separate PR for that.
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.
Isn't that changing this code to use /* @vite-ignore */
instead, or is there more to it?
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.
new URL(foo, import.meta.url)
doesn't support /* @vite-ignore */
now. So adding support for that is needed.
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.
Ah okay I thought it's already supported. Good to merge then.
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. I assume this should also work in non-esmodule workers as we have
vite/packages/vite/src/node/plugins/worker.ts
Lines 188 to 200 in badaadb
export function webWorkerPostPlugin(): Plugin { | |
return { | |
name: 'vite:worker-post', | |
resolveImportMeta(property, { chunkId, format }) { | |
// document is undefined in the worker, so we need to avoid it in iife | |
if (property === 'url' && format === 'iife') { | |
return 'self.location.href' | |
} | |
return null | |
}, | |
} | |
} |
Yep, it works by that part. |
Description
Use
import.meta.url
instead of replacing them withself.location
.fixes #10842
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).