Skip to content

refactor(rolldown_plugin_vite_import_glob): do not rewrite import path for absolute base#9195

Merged
shulaoda merged 3 commits intomainfrom
04-21-fix-do-not-rewrite-import-path-for-absolute-base
Apr 24, 2026
Merged

refactor(rolldown_plugin_vite_import_glob): do not rewrite import path for absolute base#9195
shulaoda merged 3 commits intomainfrom
04-21-fix-do-not-rewrite-import-path-for-absolute-base

Conversation

@shulaoda
Copy link
Copy Markdown
Member

Related to #9145, #9145 (comment)

Summary

  • Stop overriding import_path when options.base starts with /. Previously the code re-computed import_path = self.relative_path(file, None), which was intended to emulate Vite's /${relative(root, file)} shape but produced a broken result for files outside root (no leading /, e.g. ../basic/dir/a.js).
  • Keep import_path as self.relative_path(file, Some(dir)) — a plain path relative to the importer's directory — regardless of whether base is absolute. This yields stable, directly-resolvable relative imports that don't depend on any resolver-specific behavior.
  • Extend the import-glob/parent fixture with two new cases covering base: '/', including one where the glob reaches a sibling directory (../basic/dir/*.js) outside the importer's folder.

Why not align with Vite's importPath = \/${relative(root, file)}``?

Vite's importMetaGlob.ts at L495-L497 rewrites importPath to an absolute-looking form (e.g. /../external/a.js) and relies on its resolver to turn that back into a real file. Concretely, rolldown_plugin_vite_resolve sets:

roots: if base_options.as_src { vec![base_options.root.clone()] } else { vec![] }

oxc-resolver's roots option says: for requests starting with /, resolve them against each root (plus an absolute-path attempt on non-Windows). That is what lets Vite recover /../external/a.js — it falls through to ${root}/../external/a.js and normalizes.

This only works while asSrc is true. For oxcResolvePlugin that is effectively always the case, except inside createIdResolver, where asSrc is false and roots becomes empty. In that path, a generated /../external/a.js would fail to resolve.

Using an importer-relative path (../basic/dir/a.js) side-steps this entirely: it's a plain relative specifier that any resolver handles the same way, independent of asSrc/roots. The trade-off is an intentional shape divergence from Vite — the keys in the returned import.meta.glob object differ when base: '/' is used with out-of-dir matches — but resolution becomes correct and resolver-agnostic.

@shulaoda
Copy link
Copy Markdown
Member Author

Do you think this change is reasonable? To some extent, it diverges from Vite rather than strictly aligning with it, and instead introduces a more generalized extension. @sapphi-red

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 22, 2026

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 04546f6
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69eadd2b9115200008a6970a
😎 Deploy Preview https://deploy-preview-9195--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 22, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 04-21-fix-do-not-rewrite-import-path-for-absolute-base (04546f6) with main (a71934b)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sapphi-red
Copy link
Copy Markdown
Member

Is this a fix? or is this a refactor?

@shulaoda shulaoda changed the title fix(rolldown_plugin_vite_import_glob): do not rewrite import path for absolute base refactor(rolldown_plugin_vite_import_glob): do not rewrite import path for absolute base Apr 23, 2026
@shulaoda
Copy link
Copy Markdown
Member Author

Is this a fix? or is this a refactor?

Oh, sorry, this is more of a refactor aimed at making it extensible for third-party usage.

Copy link
Copy Markdown
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

LGTM let's update the code in Vite as well.

@shulaoda shulaoda merged commit de65659 into main Apr 24, 2026
33 checks passed
@shulaoda shulaoda deleted the 04-21-fix-do-not-rewrite-import-path-for-absolute-base branch April 24, 2026 03:37
This was referenced Apr 29, 2026
shulaoda added a commit that referenced this pull request Apr 29, 2026
## [1.0.0-rc.18] - 2026-04-29

### 💥 BREAKING CHANGES

- optimization: default unspecified inlineConst.mode to smart (#9248) by @IWANABETHATGUY

### 🐛 Bug Fixes

- rolldown_plugin_vite_import_glob: return error instead of panicking when virtual module uses a relative glob (#9241) by @shulaoda
- binding: treat empty inlineConst object as omitted (#9247) by @IWANABETHATGUY
- rolldown: keep enum declaration for optional-chain access (#9229) by @Dunqing
- link_stage: restore inline let-else in exports-kind filter (#9237) by @IWANABETHATGUY
- dev/lazy: avoid module reinitialization in lazy compilation patches (#9179) by @h-a-n-a
- dev: visit identifier references for runtime rewrites in HMR finalizer (#9191) by @h-a-n-a
- chunk-optimizer: pick dominator for runtime placement to avoid cycles (#9164) by @IWANABETHATGUY
- make `this.emitFile` chunk path synchronous to avoid deadlock (#9031) by @lazarv
- use sentinel id for `browser: false` ignored modules (#9192) by @shulaoda
- prevent chunk optimizer from creating import cycles (#9228) by @IWANABETHATGUY

### 🚜 Refactor

- replace tokio::sync::Mutex with std::sync::Mutex for non-IO data (#9176) by @shulaoda
- rolldown_plugin_vite_import_glob: do not rewrite import path for absolute base (#9195) by @shulaoda
- runtime_helper: wrap DependedRuntimeHelperMap in a struct (#9215) by @IWANABETHATGUY
- drop redundant clear() in determine_safely_merge_cjs_ns (#9206) by @IWANABETHATGUY
- clean up generate_lazy_export (#9208) by @IWANABETHATGUY
- bitset: return bool from set_bit to fuse guard-and-set (#9207) by @IWANABETHATGUY
- link_stage: simplify exports-kind filter and clarify safety comments (#9205) by @IWANABETHATGUY

### 📚 Documentation

- determine_module_exports_kind (#9252) by @IWANABETHATGUY
- fix dead link to esbuild ESM/CJS interop tests (#9230) by @Copilot
- remove CSS bundling references (#9234) by @shulaoda
- correct IncrementalFullBuild row in BundleMode table (#9214) by @IWANABETHATGUY
- design: add bundler data lifecycle design doc (#9212) by @hyf0
- remove minifier alpha status notices (#9202) by @sapphi-red

### ⚙️ Miscellaneous Tasks

- upgrade oxc to 0.128.0 (#9260) by @shulaoda
- deps: bump rolldown-ariadne to 0.6.0 (#9254) by @IWANABETHATGUY
- deps: update github actions (#9259) by @renovate[bot]
- deps: update github actions (#9258) by @renovate[bot]
- remove renovate overrides (#9257) by @Boshen
- use ubuntu-latest for security workflow (#9256) by @Boshen
- notify Discord around release publish (#9251) by @Boshen
- add release environment to npm publish workflow (#9250) by @Boshen
- justfile: drop the `--` separator before forwarded args in `vp run` (#9246) by @shulaoda
- deps: update test262 submodule for tests (#9243) by @sapphi-red
- add more tracing instrumentations (#9220) by @sapphi-red
- rolldown_plugin_vite_import_glob: remove outdated sourcemap doc comment (#9213) by @shulaoda
- update security workflow (#9201) by @Boshen

### ❤️ New Contributors

* @lazarv made their first contribution in [#9031](#9031)

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
@rolldown-guard rolldown-guard Bot mentioned this pull request Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants