Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: __VITE_PUBLIC_ASSET__hash__ in HTML #9247
fix: __VITE_PUBLIC_ASSET__hash__ in HTML #9247
Changes from 3 commits
4cd9625
2c3d85a
d1bb5e9
edb7ab2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This test is failing because during dev
await getBg('.inline-style-public')
is"url("http://localhost:5173/icon.png")"
.Vite's test runs with dev and then runs with build+preview.
I think there is a different bug here.
await getBg('.inline-style-public')
should returnurl("http://localhost:5173/foo/icon.png")
and noturl("http://localhost:5173/icon.png")"
during dev too. (becausesrc="/raw.js"
(atassets/index.html
line 18) is rewrote tosrc="/foo/raw.js"
during dev.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.
Yeah. I guess there is a bug in Vitest configuration here but I'm not familiar so I haven't catched it yet. I will investigate more. But if you spot the bug, please let me know, or just push the change directly to my branch. Much appreciated.
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.
The weird thing is that if it's class style, it works fine. The test fail with inline style only. So I am nor sure it's a bug in Vitest config or the code implementation 😂
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.
No, I meant there is a bug in Vite.
Good finding 👍 I think this is a different bug.
I haven't checked closely yet but I suppose
style
attributes needs to be rewrote here during dev.vite/packages/vite/src/node/server/middlewares/indexHtml.ts
Lines 185 to 253 in 2c3d85a
If you prefer, I think we could go with commenting out this line for now since it's a different bug. (I'll create a issue or PR later)
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.
Thank for your help. I will separate it to a different test case then skip it for now. I will push it soon.
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 just re-ran the code again in DEV mode and BUILD mode. I confirm that the
url()
in inline style is missingbase
url.Dev mode
Missing
/foo
This is what I get when I visit
http://127.0.0.1:5173/icon.png
directly.But I don't understand why it works fine as a background image in a CSS rule
Build mode
Have
/foo
So I guess we just discovered another bug.
I also pushed the commit to skip the failed test for DEV mode.