Skip to content

fix(testing): remove unintended trigger_full_build from test harness#9573

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/recover-snapshot-cleanup
May 27, 2026
Merged

fix(testing): remove unintended trigger_full_build from test harness#9573
graphite-app[bot] merged 1 commit into
mainfrom
fix/recover-snapshot-cleanup

Conversation

@hyf0

@hyf0 hyf0 commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #9552.

The trigger_full_build + ensure_latest_bundle_output composition added to ensureLatestBuildOutputForEachStep was causing unintended side effects:

  • recover_after_generate_bundle_error: duplicate Build Output errors in Step 0, lost HMR Code/Meta in Step 1
  • from_rebuild_syntax_error: extra Build Output from trigger that wasn't needed

Revert to only calling ensure_latest_bundle_output and restore snapshots to their correct state.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 26, 2026 16:02
@netlify

netlify Bot commented May 26, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 49746ca
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a16e10bd1295b00080d2a3a

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the dev-mode integration test harness to avoid unintentionally triggering extra full builds when capturing per-step build output, restoring HMR test snapshots to reflect the intended single-build behavior (follow-up to #9552).

Changes:

  • Remove the trigger_full_build + stale-output retry logic from the test harness’ per-step “ensure latest output” helper.
  • Update HMR artifact snapshots to match the corrected build/HMR callback sequencing (no duplicate build outputs; HMR code/meta preserved).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
crates/rolldown_testing/src/integration_test.rs Stops forcing a full rebuild when ensure_latest_build_output_for_each_step is enabled; only waits for ensure_latest_bundle_output.
crates/rolldown/tests/rolldown/topics/hmr/recover_after_generate_bundle_error/artifacts.snap Snapshot updated to reflect correct HMR Step 1 code/meta and non-duplicated build output.
crates/rolldown/tests/rolldown/topics/hmr/error_recovery/from_rebuild_syntax_error/artifacts.snap Snapshot updated to remove unintended extra build output and restore HMR Step 1 code/meta.

@codspeed-hq

codspeed-hq Bot commented May 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/recover-snapshot-cleanup (c2e49d6) with main (c7bbe2d)

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 commented May 27, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • May 27, 2:19 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 27, 11:05 AM UTC: hyf0 added this pull request to the Graphite merge queue.
  • May 27, 11:11 AM UTC: The Graphite merge queue couldn't merge this PR because it was in draft mode.
  • May 27, 11:13 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 27, 12:13 PM UTC: The merge label 'graphite: merge-when-ready' was removed. This PR will no longer be merged by the Graphite merge queue
  • May 27, 12:13 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 27, 12:13 PM UTC: shulaoda added this pull request to the Graphite merge queue.
  • May 27, 12:24 PM UTC: Merged by the Graphite merge queue.

@h-a-n-a h-a-n-a left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

graphite-app Bot pushed a commit that referenced this pull request May 27, 2026
…9573)

## Summary

Follow-up to #9552.

The `trigger_full_build` + `ensure_latest_bundle_output` composition added to `ensureLatestBuildOutputForEachStep` was causing unintended side effects:
- `recover_after_generate_bundle_error`: duplicate Build Output errors in Step 0, lost HMR Code/Meta in Step 1
- `from_rebuild_syntax_error`: extra Build Output from trigger that wasn't needed

Revert to only calling `ensure_latest_bundle_output` and restore snapshots to their correct state.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app Bot force-pushed the fix/recover-snapshot-cleanup branch from c2e49d6 to 4381bf4 Compare May 27, 2026 11:06
@shulaoda shulaoda marked this pull request as draft May 27, 2026 11:10
@shulaoda shulaoda marked this pull request as ready for review May 27, 2026 12:13
…9573)

## Summary

Follow-up to #9552.

The `trigger_full_build` + `ensure_latest_bundle_output` composition added to `ensureLatestBuildOutputForEachStep` was causing unintended side effects:
- `recover_after_generate_bundle_error`: duplicate Build Output errors in Step 0, lost HMR Code/Meta in Step 1
- `from_rebuild_syntax_error`: extra Build Output from trigger that wasn't needed

Revert to only calling `ensure_latest_bundle_output` and restore snapshots to their correct state.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app Bot merged commit 49746ca into main May 27, 2026
33 checks passed
@graphite-app graphite-app Bot deleted the fix/recover-snapshot-cleanup branch May 27, 2026 12:24
@rolldown-guard rolldown-guard Bot mentioned this pull request Jun 3, 2026
shulaoda added a commit that referenced this pull request Jun 3, 2026
> [!IMPORTANT]
> **This is a minor release.** Two changes alter default behavior compared to `1.0.3`. Please read this section before upgrading. Everything else is additive (new features, fixes, deps).

## ⚠️ Notable behavior changes

### 1. `experimental.lazyBarrel` is now enabled by default (#9632)

**What changed.** `experimental.lazyBarrel` now defaults to `true`. When a barrel module is recognized as side-effect-free, Rolldown skips compiling the re-exported modules that are never actually used.

**Impact.** For codebases with large barrel files (component libraries such as Ant Design, `@mui/icons-material`, etc.) this is a meaningful build-time speedup, and for the vast majority of projects the emitted output is unchanged. In rare cases where a barrel is *incorrectly* treated as side-effect-free, the optimization could drop a module that was being relied on for its side effects.

**How to opt out (backward compatible).**

```js
// rolldown.config.js
export default {
  experimental: { lazyBarrel: false },
}
```

> Note: this opt-out flag is planned to be removed in a future release. If you have a case where you must turn it off, please open an issue so we can fix the underlying detection instead.

---

### 2. `tsconfig` project-reference resolution now aligns with TypeScript

Upgrading `oxc_resolver` (`11.19.1` → `11.20.0` in #9549, then `→ 11.21.0` in #9634) changes how a *solution-style* `tsconfig.json` (one that only lists `references` and delegates the real settings to `tsconfig.app.json` / `tsconfig.node.json`, as Vite scaffolds) is resolved, bringing it **in line with how TypeScript (`tsc`) itself behaves**:

- **Reference match priority** (oxc-resolver [#1151](oxc-project/oxc-resolver#1151)): when the root has `references`, a referenced project that includes the file now **takes precedence over the root**, instead of the root matching it first (this is what TypeScript already does). So that project's `compilerOptions.paths` now apply.
- **`allowJs`** (oxc-resolver [#1198](oxc-project/oxc-resolver#1198)): whether a `.js`/`.jsx`/`.mjs`/`.cjs` file is included is now decided by **each referenced project's own** `allowJs`, not the root's (again matching TypeScript). So `tsconfig.app.json` with `allowJs: true` + `paths` now resolves aliases for `.js` files even when the root doesn't set `allowJs`.

For most projects this is a fix (the standard Vite `paths` aliases now resolve, closes #8468), but it **is** a behavior change if you relied on the previous behavior, where the root's `paths` / `allowJs` took precedence.

**If you relied on the old "root wins" behavior.** There is no exact toggle back, because the old behavior was the bug being fixed. The recommended path is to align your config with TypeScript: declare the `paths` / `allowJs` on the referenced project that actually owns the files.

If you must keep the old precedence while still using `references`: a referenced project's match wins, and **the first matching `references` entry takes priority** (the root is only a fallback when no reference claims the file). So extract the old root settings into their own config and list it **first**:

```jsonc
// tsconfig.json (solution root)
{
  "files": [],
  "references": [
    { "path": "./tsconfig.base.json" }, // old root paths/allowJs — listed first, so it wins
    { "path": "./tsconfig.app.json" },
    { "path": "./tsconfig.node.json" }
  ]
}
```

`tsconfig.base.json` should carry the `paths` you previously declared on the root, plus `allowJs: true` if it needs to claim `.js` files (the extension is checked against each config's own `allowJs`). With no `include`, it defaults to `**/*` under its directory and claims every file first.

Alternatively, bypass reference resolution entirely by pointing the top-level `tsconfig` option at a single config: `export default { tsconfig: './tsconfig.app.json' }`.

---

## [1.1.0] - 2026-06-03

### 🚀 Features

- enable `experimental.lazyBarrel` by default (#9632) by @shulaoda
- `import.meta.glob` support `caseSensitive` option (#9594) by @btea
- add `SOURCEMAP_BROKEN` warning for renderChunk hook (#9601) by @sapphi-red
- add `SOURCEMAP_BROKEN` warning for transform hook (#9600) by @sapphi-red
- add `@__NO_SIDE_EFFECTS__` hint for invalid `@__PURE__` before function declarations (#9505) by @Copilot
- code-splitting: support group-local `includeDependenciesRecursively` (#9587) by @hyf0

### 🐛 Bug Fixes

- report TSCONFIG_ERROR instead of UNHANDLEABLE_ERROR for a missing tsconfig file (#9633) by @shulaoda
- browser: add missing exports and ensure consistency with `rolldown` package (#9629) by @sapphi-red
- should build test-dev-server when test-node (#9610) by @situ2001
- chunk-optimizer: refuse asymmetric merge for cyclic dynamic entries (#9320) (#9322) by @aminpaks
- dev: handle the remaining errors in dev (#9570) by @h-a-n-a
- handle slash-normalized ids with preserveModulesRoot (#9595) by @IWANABETHATGUY
- json: preserve .default access on JSON default imports (#9568) by @IWANABETHATGUY
- testing: remove unintended trigger_full_build from test harness (#9573) by @hyf0

### 🚜 Refactor

- js-regex: use regress native replace/replace_all (#9607) by @IWANABETHATGUY
- remove never-constructed `ImportStatus` variants (#9606) by @Boshen

### 📚 Documentation

- clarify that `RolldownBuild::close` method should be called in most cases (#9619) by @sapphi-red

### ⚡ Performance

- avoid unnecessary intermediate sourcemaps (#9599) by @sapphi-red

### 🧪 Testing

- add unit test for collapsing module sourcemap (#9626) by @sapphi-red
- cover vite-alias regex capture-group expansion (#9602) (#9608) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- deps: update oxc_resolver to 11.21.0 (#9634) by @shulaoda
- update invalid option diagnostic link to point to Rolldown docs (#9631) by @sapphi-red
- deps: update vite+ to v0.1.24 (#9628) by @renovate[bot]
- deps: update oxc resolver to v11.20.0 (#9549) by @renovate[bot]
- deps: update dependency vite-plus to v0.1.24 (#9470) by @renovate[bot]
- deps: update npm packages (#9614) by @renovate[bot]
- deps: upgrade oxc to 0.134.0 (#9625) by @shulaoda
- deps: update crate-ci/typos action to v1.47.0 (#9620) by @renovate[bot]
- deps: update rollup submodule for tests to v4.61.0 (#9623) by @rolldown-guard[bot]
- deps: update github actions (#9613) by @renovate[bot]
- deps: update pnpm to v11.4.0 (#9616) by @renovate[bot]
- deps: update rust crates (#9615) by @renovate[bot]
- deps: update test262 submodule for tests (#9624) by @rolldown-guard[bot]
- deps: update dependency @napi-rs/cli to v3.7.0 (#9588) by @renovate[bot]
- deps: update dependency rust to v1.96.0 (#9596) by @renovate[bot]
- re-enable WASI testing with proper infrastructure (#9397) by @Boshen

### ❤️ New Contributors

* @aminpaks made their first contribution in [#9322](#9322)

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.

4 participants