fix(asset): resolve dynamic new URL paths without a leading ./ (fix #10032) - #23433
Open
kakiuwang-ui wants to merge 1 commit into
Open
fix(asset): resolve dynamic new URL paths without a leading ./ (fix #10032)#23433kakiuwang-ui wants to merge 1 commit into
new URL paths without a leading ./ (fix #10032)#23433kakiuwang-ui wants to merge 1 commit into
Conversation
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
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.
Fixes #10032.
new URL('pets/cat.txt', import.meta.url)andnew URL('./pets/cat.txt', import.meta.url)resolve to the same file, because the first argument ofnew URL()is always resolved against the base URL — it is never a bare specifier. The template literal form does not agree:new URL(`pets/${name}.txt`, import.meta.url)is turned intoimport.meta.glob("pets/*.txt", ...), andtoAbsoluteGlobrejects a pattern that starts with neither/nor./.The symptom has changed since the issue was filed, and both modes are now worse than the build error that was reported:
vite devthrowsInvalid glob: "pets/*.txt", so the module 500s.vite buildonly warns and emits the untransformed call into the bundle:new URL(import.meta.glob(`pets/*.txt`, {...})[`pets/${e}.txt`], import.meta.url).import.meta.globdoes not exist at runtime, so the chunk throws aTypeErrorin the browser. Reverting this patch and runningplayground/assetsshows the blast radius: 144 failures in serve mode and 5 in build mode, most of them neighbours taken down by the one broken module.Implementation
assetImportMetaUrlnow prefixes./to both the glob pattern and the runtime index key (they have to stay in sync, or the lookup returnsundefinedsilently).The prefix is only added when nothing else can claim the specifier, so that the pattern resolves in the same order as the static branch below it (alias, then relative, then bare):
alias: { pets: './other/pets' }and a realsrc/pets/, the static form resolves toother/petsbecause@rollup/plugin-aliasruns ahead of the oxc resolver andpreferRelativeonly applies after it. An unconditional prefix would make the dynamic form picksrc/petsinstead — trading one inconsistency for another. The check is analiasOnlyresolve, and it only runs when the sibling directory actually exists, so it is not on the path of every dynamicnew URL.#-prefixed patterns are left alone;toAbsoluteGlobhas explicit subpath-imports handling for them.process.cwd().Invalid globerror.assetResolver's lazy initialisation is pulled intogetAssetResolver()so the two call sites share it.Notes for review
globBaseExistsusesstatSync, which is case-insensitive on macOS and Windows, while the glob runs withcaseSensitiveMatch: true.new URL(`FOO/${n}.js`, import.meta.url)next to afoo/directory is therefore rewritten and then matches nothing, where it used to raiseInvalid glob. I left it that way on purpose:`./FOO/${n}.js`already fails silently today, and making only the un-prefixed form loud would recreate the inconsistency this PR removes. Happy to add a case-exact check if you would rather have the error back.tryFsResolveplus awarnOncefallback), so I did not add a watch for it here.new URL(`@/${n}.png`, import.meta.url)produces keys like/src/foo.pngwhile the lookup uses`@/${n}.png`and evaluates toundefined. That is pre-existing and unchanged here; happy to open a separate issue or PR.import.meta.globcall leaking into the production bundle instead of failing comes frombuiltin:vite-import-globwarning where the JS plugin throws. This PR removes the case that reaches it, but the leak itself is still there for alias and bare patterns.Test
packages/vite/src/node/__tests__/plugins/assetImportMetaUrl.spec.tscovers the rewrite plus each of the four cases that must not be rewritten. The transform helper now takes an importer, because the check needs a real directory; the new cases point atfixtures/asset-import-meta-url/.I checked the tests discriminate rather than just pass, by breaking the implementation one guard at a time and confirming each failure lands on the intended assertion: dropping the existence check fails the bare-specifier case, prefixing the pattern without the index key fails the relative case, and dropping the
#,path.isAbsoluteand alias guards each fail their own case.playground/assetsgets a dynamicnew URLwithout a leading./, which fails in serve, build and bundled-dev without this patch.