Skip to content

fix(plugin): stop this.resolve recursion between plugins with skipSelf: true - #11181

Merged
sapphi-red merged 4 commits into
rolldown:mainfrom
anzal1:fix/resolve-recursive-skip-self
Oct 21, 2026
Merged

sapphi-red merged 4 commits into
rolldown:mainfrom
anzal1:fix/resolve-recursive-skip-self

Conversation

@anzal1

@anzal1 anzal1 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

This fixes this.resolve recursion between two plugins that never terminates. There is no tracking issue. The case is Rollup's resolveid-recursive-call test (rollup/rollup#5945), which was listed in packages/rollup-tests/src/failed-tests.json.

Root cause

this.resolve with skipSelf only filtered plugins at dispatch time. If plugin r1 calls this.resolve(id, 'foo') and plugin r2 calls this.resolve(id, 'bar'), r1 gets called again with importer bar and calls this.resolve(id, 'foo') again. The skip list already holds (r1, id, 'foo'), but nothing checks it, so the chain repeats forever.

Fix

In NativePluginContextImpl::resolve, when skipSelf is on and the skip list already has an entry for this plugin with the same specifier and importer, return a not-found result. The node binding turns that into null, so the plugin falls back to its own default. This mirrors resolveIdViaPlugins in Rollup, which returns null with the comment "plugin recursively called itself".

Tests

  • New fixture packages/rolldown/tests/fixtures/plugin/context/resolve-recursive-skip-self. It throws after 20 calls so a regression fails fast instead of hanging. Before the fix it fails with "resolve recursion did not terminate". After the fix it passes with the call order r1, r2, r1.
  • Removed rollup@function@resolveid-recursive-call from failed-tests.json and refreshed status.json and status.md. The Rollup suite passes with the entry removed.
  • cargo test -p rolldown_plugin and cargo clippy -p rolldown_plugin pass.

AI usage

This PR was written with the help of an AI agent (Claude Code). The change is a small bug fix with clear expected behavior, matching Rollup.


Written with the help of an agent (Claude Code, claude-opus-5-5 orchestrating claude-sonnet-5-5).

@anzal1
anzal1 marked this pull request as ready for review October 16, 2026 18:46
@anzal1
anzal1 force-pushed the fix/resolve-recursive-skip-self branch from 3684ca3 to 110c442 Compare October 19, 2026 06:27
@codspeed

codspeed Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 4 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing anzal1:fix/resolve-recursive-skip-self (ad3305a) with main (5bfb74e)

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 sapphi-red changed the title fix(plugin): stop this.resolve recursion between plugins with skipSelf fix(plugin): stop this.resolve recursion between plugins with skipSelf: true Oct 21, 2026
@sapphi-red
sapphi-red enabled auto-merge (squash) October 21, 2026 08:33
@sapphi-red
sapphi-red merged commit 889799e into rolldown:main Oct 21, 2026
32 checks passed
@rolldown-guard rolldown-guard Bot mentioned this pull request Oct 21, 2026
shulaoda added a commit that referenced this pull request Oct 21, 2026
## [1.2.15] - 2026-10-21

### 🚀 Features

- rolldown_plugin_vite_resolve: add `isBundled` to pick the browser-external stub format (#11260) by @h-a-n-a
- dev: mark entry modules in the HMR module graph (#11252) by @h-a-n-a
- mark module starts with `// <id>` comments instead of `#region` pairs (#11235) by @IWANABETHATGUY

### 🐛 Bug Fixes

- plugin: stop `this.resolve` recursion between plugins with `skipSelf: true` (#11181) by @anzal1
- rolldown_plugin_vite_resolve: handle external `require` for `ssr.target: 'webworker'` (#11205) by @sapphi-red
- rolldown_plugin_vite_build_import_analysis: only wrap dynamic imports with static specifiers in __vitePreload (#10879) by @smeng9
- strict-execution-order: wrap eager modules that forward wrapped definers (#11186) by @hyfdev
- chunk-import-map: keep common chunk keys unique when enabling inline common chunks (#11230) by @hyfdev
- only register signal-exit handler when a trace subscriber is active (#11208) by @DaniilIsupov

### 🧪 Testing

- fold hand-unrolled Rust test tables (#11212) by @Brooooooklyn

### ⚙️ Miscellaneous Tasks

- deps: upgrade oxc to 0.155.0 (#11228) by @Dunqing
- rollup-tests: retriage failed tests (#11251) by @sapphi-red
- deps: update rollup submodule for tests to v4.64.12 (#11249) by @rolldown-guard[bot]
- deps: update test262 submodule for tests (#11242) by @rolldown-guard[bot]

### ❤️ New Contributors

* @anzal1 made their first contribution in [#11181](#11181)
* @smeng9 made their first contribution in [#10879](#10879)
* @DaniilIsupov made their first contribution in [#11208](#11208)

Co-authored-by: shulaoda <[email protected]>
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.

2 participants