Skip to content

fix(optimizer): handle more chars that will be sanitized#22208

Open
sapphi-red wants to merge 2 commits intovitejs:mainfrom
sapphi-red:fix/optimize-deps-handle-more-chars-that-will-be-sanitized
Open

fix(optimizer): handle more chars that will be sanitized#22208
sapphi-red wants to merge 2 commits intovitejs:mainfrom
sapphi-red:fix/optimize-deps-handle-more-chars-that-will-be-sanitized

Conversation

@sapphi-red
Copy link
Copy Markdown
Member

Since $ is also sanitized by Rolldown by sanitizeFileName, the filename does not contain $.
#21886 fixed a similar issue for +, but it was not a general fix. In this PR, I've rewritten the flattenId function to escape any characters that will be escaped by the default sanitizeFileName.

close #22198

@sapphi-red sapphi-red added p2-edge-case Bug, but has workaround or limited in scope (priority) feat: deps optimizer Esbuild Dependencies Optimization labels Apr 10, 2026
"version": 3,
},
visualization: "https://evanw.github.io/source-map-visualization/#MjQ3AGNvbnN0IGZvbyA9IF9fdml0ZV9fY2pzSW1wb3J0MF9fdml0ZWpzX3Rlc3RJbXBvcnRlZVBrZ1siZm9vIl07Ly8gcHJldHRpZXItaWdub3JlCmltcG9ydCBfX3ZpdGVfX2Nqc0ltcG9ydDBfX3ZpdGVqc190ZXN0SW1wb3J0ZWVQa2cgZnJvbSAiL25vZGVfbW9kdWxlcy8udml0ZS9kZXBzL0B2aXRlanNfdGVzdC1pbXBvcnRlZS1wa2cuanM/dj0wMDAwMDAwMCI7CmNvbnNvbGUubG9nKCJ3aXRoLW11bHRpbGluZS1pbXBvcnQiLCBmb28pOwoyNDgAeyJtYXBwaW5ncyI6IjtBQUNBLFNBQ0UsV0FDSztBQUVQLFFBQVEsSUFBSSx5QkFBeUIsSUFBSSIsInNvdXJjZXMiOlsid2l0aC1tdWx0aWxpbmUtaW1wb3J0LnRzIl0sInZlcnNpb24iOjMsInNvdXJjZXNDb250ZW50IjpbIi8vIHByZXR0aWVyLWlnbm9yZVxuaW1wb3J0IHtcbiAgZm9vXG59IGZyb20gJ0B2aXRlanMvdGVzdC1pbXBvcnRlZS1wa2cnXG5cbmNvbnNvbGUubG9nKCd3aXRoLW11bHRpbGluZS1pbXBvcnQnLCBmb28pXG4iXX0="
visualization: "https://evanw.github.io/source-map-visualization/#MjQ5AGNvbnN0IGZvbyA9IF9fdml0ZV9fY2pzSW1wb3J0MF9fdml0ZWpzX3Rlc3RJbXBvcnRlZVBrZ1siZm9vIl07Ly8gcHJldHRpZXItaWdub3JlCmltcG9ydCBfX3ZpdGVfX2Nqc0ltcG9ydDBfX3ZpdGVqc190ZXN0SW1wb3J0ZWVQa2cgZnJvbSAiL25vZGVfbW9kdWxlcy8udml0ZS9kZXBzL0B2aXRlanNfMmZ0ZXN0LWltcG9ydGVlLXBrZy5qcz92PTAwMDAwMDAwIjsKY29uc29sZS5sb2coIndpdGgtbXVsdGlsaW5lLWltcG9ydCIsIGZvbyk7CjI0OAB7Im1hcHBpbmdzIjoiO0FBQ0EsU0FDRSxXQUNLO0FBRVAsUUFBUSxJQUFJLHlCQUF5QixJQUFJIiwic291cmNlcyI6WyJ3aXRoLW11bHRpbGluZS1pbXBvcnQudHMiXSwidmVyc2lvbiI6Mywic291cmNlc0NvbnRlbnQiOlsiLy8gcHJldHRpZXItaWdub3JlXG5pbXBvcnQge1xuICBmb29cbn0gZnJvbSAnQHZpdGVqcy90ZXN0LWltcG9ydGVlLXBrZydcblxuY29uc29sZS5sb2coJ3dpdGgtbXVsdGlsaW5lLWltcG9ydCcsIGZvbylcbiJdfQ=="
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sourcemap changes are caused by the optimized filename change.

Comment thread packages/vite/src/node/utils.ts Outdated
@afurm
Copy link
Copy Markdown

afurm commented Apr 11, 2026

The approach of aligning flattenId with sanitizeFileName is the right direction — handling a class of invalid path chars instead of enumerate-and-replace is more maintainable.

One concern: flattenId escapes _ to __ before calling sanitizeFileName, but then sanitizeFileName decodes __2f_ and __2b+. This means a dependency ID that legitimately contains the substring _2f (e.g. my_2f_lib) would after pre-bundling produce the same flattened ID as my/lib. Does sanitizeFileName have a mechanism to avoid this ambiguity, or is there an ordering issue where flattenId needs to run after sanitizeFileName rather than before?

Copy link
Copy Markdown
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and I think this should invalidate the metadata hash so it's reoptimized on Vite update. Otherwise it might still refer to the old paths.

Comment on lines +76 to +83
.replaceAll('_', '__')
// replace any characters that will be replaced by sanitizeFileName
.replace(invalidUrlPathCharRE, (c) => '_' + c.charCodeAt(0).toString(16))
.replace(
additionalFlattenIdCharRE,
(c) => '_' + c.charCodeAt(0).toString(16),
)
.replace(replaceNestedIdRE, '__'),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _2f pattern here makes it harder to read personally. I understand making it robust, but maybe we can go with a middleground:

What if we do the normal underscore replacements for the commons chars, like / and ., and then do the _2f pattern for the rest? Maybe even _2f_ so it's slightly easier to read.

Also, it'd be nice to have some more tests for the new chars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat: deps optimizer Esbuild Dependencies Optimization p2-edge-case Bug, but has workaround or limited in scope (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Vite 8] Error when importing package with $ symbol

3 participants