Skip to content

fix(rolldown_plugin_vite_import_glob): import path should not be affected by absolute base option#9145

Merged
shulaoda merged 2 commits intorolldown:mainfrom
kermanx:fix/9144
Apr 22, 2026
Merged

fix(rolldown_plugin_vite_import_glob): import path should not be affected by absolute base option#9145
shulaoda merged 2 commits intorolldown:mainfrom
kermanx:fix/9144

Conversation

@kermanx
Copy link
Copy Markdown
Contributor

@kermanx kermanx commented Apr 17, 2026

fix #9144

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 17, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing kermanx:fix/9144 (19f1e06) with main (75bb10b)

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.

@hyf0 hyf0 requested a review from shulaoda April 17, 2026 04:29
let file_path = if let Some(base) = &options.base {
if base.starts_with('/') {
import_path = self.relative_path(file, None);
import_path = format!("/{}", file.relative(self.root).to_slash_lossy());
Copy link
Copy Markdown
Member

@shulaoda shulaoda Apr 22, 2026

Choose a reason for hiding this comment

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


export default defineTest({
// Skipping this test for now to align with vite
skip: true,
Copy link
Copy Markdown
Member

@shulaoda shulaoda Apr 22, 2026

Choose a reason for hiding this comment

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

This is because it generates imports like () => import('/../external/a.js'). In Vite, this is handled by the resolver via roots: [root], but Rolldown’s resolver does not currently handle this case. I will try to extend it later to support this.

@shulaoda shulaoda merged commit 0691d6f into rolldown:main Apr 22, 2026
28 checks passed
@shulaoda
Copy link
Copy Markdown
Member

Thanks for your contribution. Your original fix is correct, but for now we’d prefer to align with Vite first, and then discuss whether we should extend the behavior afterward.

IWANABETHATGUY pushed a commit that referenced this pull request Apr 22, 2026
…cted by absolute base option (#9145)

fix #9144

---------

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
This was referenced Apr 22, 2026
shulaoda added a commit that referenced this pull request Apr 22, 2026
## [1.0.0-rc.17] - 2026-04-22

### 🐛 Bug Fixes

- link: error on missing export between TS modules (#9197) by @IWANABETHATGUY
- rolldown_plugin_vite_import_glob: import path should not be affected by absolute base option (#9145) by @kermanx
- `this.resolve()` returns null for bare relative paths without importer (#9142) by @Copilot
- collect destructured bindings in HMR module exports (#9146) by @h-a-n-a
- esbuild-tests: handle 0.28.0 test cases (#9149) by @sapphi-red
- plugin/copy-module: honor external resolutions from other plugins (#9139) by @TheAlexLichter
- allow undefined in sourcesContent type (#9136) by @jurijzahn8019
- reduce false positives in chunk optimizer circular dependency detection (#9049) by @AlonMiz

### 🚜 Refactor

- chunk-optimizer: extract runtime-module placement into rehome_runtime_module (#9163) by @IWANABETHATGUY

### 📚 Documentation

- add design doc for sort_modules execution ordering (#9169) by @IWANABETHATGUY
- add document for `RenderedModule` (#9147) by @sapphi-red

### ⚡ Performance

- rolldown_plugin_vite_import_glob: skip self-import earlier using raw path comparison (#9193) by @shulaoda

### 🧪 Testing

- lazy: add `playground/lazy-compilation` (#7974) by @hyf0

### ⚙️ Miscellaneous Tasks

- use app token for release PR (#9198) by @Boshen
- upgrade oxc to 0.127.0 (#9194) by @Dunqing
- use oxc security action (#9196) by @Boshen
- esbuild-tests: remove some tests from ignored list as enum inline is now supported (#9184) by @sapphi-red
- deps: update dependency vite-plus to v0.1.19 (#9183) by @renovate[bot]
- use vp instead of pnpm in check-wasi-binding-deps (#9182) by @shulaoda
- verify wasm32-wasi binding deps match @rolldown/browser before publish (#9162) by @shulaoda
- deps: update esbuild for tests to 0.28.0 (#9172) by @sapphi-red
- deps: update rollup submodule for tests to v4.60.2 (#9173) by @sapphi-red
- deps: update test262 submodule for tests (#9174) by @sapphi-red
- sort_modules: fix stale async-entry sort key comment (#9170) by @IWANABETHATGUY
- deps: update npm packages (#9157) by @renovate[bot]
- deps: update dependency diff to v9 (#9158) by @renovate[bot]
- deps: update rust crates (#9156) by @renovate[bot]
- run Windows CI on PRs labeled with `ci: windows` (#9153) by @hyf0
- update-test-dependencies: run setup-rust before file changes (#9151) by @sapphi-red
- deps: update dependency rust to v1.95.0 (#9140) by @renovate[bot]

### ❤️ New Contributors

* @jurijzahn8019 made their first contribution in [#9136](#9136)
* @AlonMiz made their first contribution in [#9049](#9049)

---------

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
shulaoda added a commit that referenced this pull request Apr 24, 2026
…h for absolute base (#9195)

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`](https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/importMetaGlob.ts#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:

```rust
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.
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.

[Bug]: import.meta.glob with absolute base resolves to wrong import path

2 participants