fix(asset): preserve typeof when rewriting asset URL placeholder (fix #22304) - #22330
Closed
mvanhorn wants to merge 1 commit into
Closed
fix(asset): preserve typeof when rewriting asset URL placeholder (fix #22304)#22330mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
…itejs#22304) When the asset placeholder is wrapped in a string literal and the literal sits to the right of a unary operator (typeof, void, !), the bare `"+runtime+"` substitution produces output that parses as `(typeof "") + runtime + ""` because typeof binds tighter than `+`. The result is a string concatenation instead of `"string"`. Expand the rewrite range to include the surrounding matching quotes and emit a parenthesized string expression `(""+runtime+"")` so a leading unary operator binds across the whole expression. Apply to both `assetUrlRE` and `publicAssetUrlRE` branches in `renderAssetUrlInJS`. Non-string contexts (CSS-in-JS like `url(__VITE_ASSET__abc__)`) keep the original substitution.
Closed
7 tasks
Member
|
I'm closing this PR as I've made a more robust solution in #22888. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Fixes #22304.
When
renderAssetUrlInJSrewrites a__VITE_ASSET__placeholder that sits inside a string literal, the bare"+runtime+"substitution closes the literal, concatenates the runtime expression, then reopens the literal. That works when the surrounding context is a binary expression likevar x = "...", but breaks when the literal sits to the right of a unary operator. With a relative base, source like:becomes:
JavaScript parses this as
(typeof "") + url + ""becausetypeofbinds tighter than+. The branch silently drops because the comparison never holds.Fix
Expand the rewrite range to include the surrounding matching quotes (single, double, or template) and emit a parenthesized string expression
(""+runtime+""). A leadingtypeof(or any unary operator) now applies to the whole concatenated string. Apply the same change to both theassetUrlREandpublicAssetUrlREbranches.Non-string contexts (CSS-in-JS like
var s = ".foo{background:url(__VITE_ASSET__abc__)}") are unchanged because the placeholder is not adjacent to matching quotes.Tests
Added unit tests in
packages/vite/src/node/__tests__/plugins/asset.spec.tscovering:typeof "..."wrapping (the regression)"...",'...', and`...`quote pairspnpm run test-unit(783 tests) andpnpm run test-build assets(218 tests across the assets playgrounds, includingrelative-base) both pass.