Repository navigation
Conversation
`new URL()` treats its first argument as a URL, so `./my%20file.txt` refers to `my file.txt` at runtime, but rolldown resolved the escaped text and failed with UNRESOLVED_IMPORT. Decode the path the way `decodeURI` does before resolving it, leaving the query, the fragment and escapes of reserved characters as written. Refs rolldown#11325
Merging this PR will not alter performance1 warning
Comparing Footnotes
|
|
|
|
%5C stay encoded too?
|
…11420) Fixes case 3 of #11325. Case 1 is #11418 and case 2 is #11419; this one is independent of both. ## Problem With `experimental.resolveNewUrlToAsset`, the query and fragment were dropped when `new URL()` was rewritten: ```js new URL("./sprite.svg?v=1", import.meta.url) // -> new URL("assets/sprite-[hash].svg", import.meta.url) new URL("./sprite.svg#icon", import.meta.url) // -> new URL("assets/sprite-[hash].svg", import.meta.url) ``` so `url.search` and `url.hash` were lost, which breaks uses like SVG sprite fragments. ## Fix In the `new URL()` finalizer, after computing the asset path, append the part of the original specifier that follows the path (everything from the first `?` or `#`, using `clean_url`). This matches Vite's `vite:asset-import-meta-url`, which keeps the postfix when it rewrites the URL. Resolution and the emitted asset are unchanged: all of `?v=1`, `#icon`, `?v=1#icon` and the plain path still share one emitted file. ## Tests New fixture `topics/new_url/keep_query_and_hash` covering a query, a fragment, both, and the plain path. With snapshot updates disabled (`INSTA_UPDATE=no CI=true`) it fails on `main` and passes here. No existing snapshot changed. `cargo test -p rolldown` passes except `test262_module_code`, which fails the same way on `main` locally because the `test262` submodule is not checked out. `cargo fmt --check` and `cargo clippy -p rolldown --all-targets` are clean. AI disclosure: I used Claude Code to help trace the code path and write the change and test. I reviewed the change and ran the tests above myself. Co-authored-by: Yunfei He <i.heyunfei@gmail.com>
Fixes case 2 of #11325. Case 1 is #11418, case 3 is a separate PR.
Problem
new URL()treats its first argument as a URL, so at runtimenew URL("./my%20file.txt", import.meta.url)refers tomy file.txt. Withexperimental.resolveNewUrlToAsset, rolldown passed the escaped text to the resolver and failed:Fix
The scanner now decodes the path before adding the import record, using a new
rolldown_utils::url::decode_uri_paththat followsdecodeURI:?queryand#fragmentare kept as written;%2F,%3F,%23and the rest ofdecodeURI's reserved set) stay escaped, so%2Fcannot turn into a path separator;%zzare kept, and if the decoded bytes are not valid UTF-8 the specifier is used unchanged.The rewritten output points at the emitted asset as before.
One behavior change worth flagging: a file whose name literally contains
%20(for examplea%20b.txt) and is referenced as./a%20b.txtwas resolved before and is not now. That matches what the browser would request at runtime without bundling, so I think it is the right reading, but say if you would rather fall back to the raw path when the decoded one does not resolve.Tests
topics/new_url/percent_encoded_pathwith./my%20file.txtand./%E2%9C%93.txt. It fails onmainwithUNRESOLVED_IMPORTfor both.decode_uri_pathinrolldown_utils.cargo test -p rolldownpasses excepttest262_module_code, which fails the same way onmainlocally because thetest262submodule is not checked out.cargo test -p rolldown_utils,cargo fmt --checkandcargo clippyare clean.AI disclosure: I used Claude Code to help trace the code path and write the change and tests. I reviewed the change and ran the tests above myself.