Skip to content

test(dev): expect a hot update across a circular import - #10700

Merged
graphite-app[bot] merged 1 commit into
mainfrom
test/circular-hot-update
Aug 18, 2026
Merged

graphite-app[bot] merged 1 commit into
mainfrom
test/circular-hot-update

Conversation

@hyfdev

@hyfdev hyfdev commented Aug 18, 2026 •

Copy link
Copy Markdown
Member

Fix dev test ci error found in #10699

The dev-server harness builds whatever vitejs/vite rolldown-canary rebased onto main gives it, so this arrived without any rolldown change: the last green run is 06:12Z and the first red one is 07:39Z, with vitejs/vite#23259 landing at 07:26Z between them. Every Node Dev Server Test job has been red since, on every branch and every Node version, including main.

The playground

main.js  →  a.js  →  b.js  ⇄  c.js
   ↑                    b and c import each other
   └── only main calls import.meta.hot.accept()

What changed upstream

Editing c.js makes the HMR client walk up the importers looking for a module that accepts updates:

c.js → imported by b; b does not accept, keep going
b.js → imported by a and by c
         via a: a → main, and main accepts ✓
         via c: that is where the walk started, it loops back

It used to drop the whole walk the moment it looped back — including the main boundary it had already found — and reload the page:

full reload needed: circular import chain between `b.js` and `c.js`

vitejs/vite#23259 makes it skip the path it already walked and keep the boundary it found, so the edit is applied through main and the page stays. Vite's unbundled dev has behaved this way since vitejs/vite#14867; bundled dev was the outlier.

Why the test broke

hmr-accept-outside-circular pinned the old behavior: wait for that log line, then assert the page reloaded (a marker planted on window is gone). Neither holds now — the log is deleted, so the wait times out with Timeout waiting for browser logs, and the marker survives.

The spec now asserts the hot update: the edit renders and the marker survives. The marker also carries the original guarantee, which the log used to carry — if the walk missed main's boundary entirely, the page would reload and wipe it, so a "content is fresh" assertion alone cannot pass by accident.

Validation: the full browser suite passes locally against Vite main at c0f2fc607, which contains the change — 104 passed / 12 skipped, exactly CI's counts with the one failure flipped. Not verified: the new assertion has not been run against a pre-#23259 Vite.

@netlify

netlify Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 04eaa68
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a84657b1f093b000878d895

@hyfdev
hyfdev force-pushed the test/circular-hot-update branch 2 times, most recently from f9bddb6 to 20ed763 Compare August 18, 2026 13:23
@hyfdev
hyfdev marked this pull request as ready for review August 18, 2026 13:59
@hyfdev
hyfdev requested a review from sapphi-red as a code owner August 18, 2026 13:59
Copilot AI lite review requested due to automatic review settings August 18, 2026 13:59

hyfdev commented Aug 18, 2026 •

Copy link
Copy Markdown
Member Author

Merge activity

  • Aug 18, 1:59 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.
  • Aug 18, 1:59 PM UTC: hyfdev added this pull request to the Graphite merge queue.
  • Aug 18, 2:08 PM UTC: Merged by the Graphite merge queue.

Fix dev test ci error found in #10699

The dev-server harness builds whatever `vitejs/vite` `rolldown-canary` rebased onto `main` gives it, so this arrived without any rolldown change: the last green run is 06:12Z and the first red one is 07:39Z, with vitejs/vite#23259 landing at 07:26Z between them. Every `Node Dev Server Test` job has been red since, on every branch and every Node version, including `main`.

### The playground

```
main.js  →  a.js  →  b.js  ⇄  c.js
   ↑                    b and c import each other
   └── only main calls import.meta.hot.accept()
```

### What changed upstream

Editing `c.js` makes the HMR client walk up the importers looking for a module that accepts updates:

```
c.js → imported by b; b does not accept, keep going
b.js → imported by a and by c
         via a: a → main, and main accepts ✓
         via c: that is where the walk started, it loops back
```

It used to drop the whole walk the moment it looped back — including the `main` boundary it had already found — and reload the page:

```
full reload needed: circular import chain between `b.js` and `c.js`
```

vitejs/vite#23259 makes it skip the path it already walked and keep the boundary it found, so the edit is applied through `main` and the page stays. Vite's unbundled dev has behaved this way since vitejs/vite#14867; bundled dev was the outlier.

### Why the test broke

`hmr-accept-outside-circular` pinned the old behavior: wait for that log line, then assert the page reloaded (a marker planted on `window` is gone). Neither holds now — the log is deleted, so the wait times out with `Timeout waiting for browser logs`, and the marker survives.

The spec now asserts the hot update: the edit renders and the marker survives. The marker also carries the original guarantee, which the log used to carry — if the walk missed `main`'s boundary entirely, the page would reload and wipe it, so a "content is fresh" assertion alone cannot pass by accident.

Validation: the full browser suite passes locally against Vite `main` at c0f2fc607, which contains the change — 104 passed / 12 skipped, exactly CI's counts with the one failure flipped. Not verified: the new assertion has not been run against a pre-#23259 Vite.
@graphite-app
graphite-app Bot force-pushed the test/circular-hot-update branch from 20ed763 to 04eaa68 Compare August 18, 2026 14:00

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

Updates the hmr-accept-outside-circular dev-server E2E spec to match Vite bundled-dev’s new circular-import HMR propagation behavior (per vitejs/vite#23259), preventing CI failures caused by expecting an obsolete “full reload” outcome.

Changes:

  • Replace the prior “wait for circular-chain full reload log + assert reload” expectation with a hot-update expectation (DOM updates to cc while the reload marker survives).
  • Remove the now-invalid log-driven synchronization (untilBrowserLogAfter) from this spec.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@graphite-app
graphite-app Bot merged commit 04eaa68 into main Aug 18, 2026
35 checks passed
@graphite-app
graphite-app Bot deleted the test/circular-hot-update branch August 18, 2026 14:08
@rolldown-guard rolldown-guard Bot mentioned this pull request Aug 19, 2026
shulaoda added a commit that referenced this pull request Aug 19, 2026
## [1.2.5] - 2026-08-19

### 🚀 Features

- bench: record per-suite peak memory in the node benchmark (#10704) by @IWANABETHATGUY
- binding: allocation-tracking global allocator behind the `tracking_allocator` feature (#10703) by @IWANABETHATGUY
- add armv7 android (armv7-linux-androideabi) support (#10691) by @shulaoda
- add `NAMESPACE_CONFLICT` warning for conflicting star re-exports (#7452) by @AliceLanniste

### 🐛 Bug Fixes

- rolldown_plugin_vite_reporter: avoid ANSI erase-line escape in non-TTY output (#10692) by @shulaoda
- rolldown_plugin_vite_resolve: preserve Yarn PnP virtual importer (#10591) by @Freakazo
- exclude hash placeholders from case-insensitive filename deconfliction (#10590) by @Nic-Polumeyv
- code-splitting: fold already-loaded side-effectful libraries into eager entries (#10645) by @JoviDeCroock
- dev: flush re-emitted assets when their content changes (#10637) by @btea
- renamer: rename nested `require`/`__filename`/`__dirname` bindings in CJS output (#10655) by @marcoroth
- silence two wasm-only warnings (#10668) by @IWANABETHATGUY

### 🚜 Refactor

- skip case folding for filenames with hash placeholders (#10689) by @hyfdev
- code-splitting: remove redundant synthetic statement owner (#10503) by @hyfdev
- renamer: move the cjs check into `rename_bindings_shadowing_cjs_ambient_names` (#10672) by @IWANABETHATGUY

### 📚 Documentation

- agents: point the never-edit list at the binding files that exist (#10719) by @melbinjp

### 🧪 Testing

- deconflict: pin renaming the external process binding away from the global (#10702) by @hyfdev
- dev: expect a hot update across a circular import (#10700) by @hyfdev

### ⚙️ Miscellaneous Tasks

- deps: upgrade oxc to 0.146.0 (#10707) by @Boshen
- deps: update rust crates (#10684) by @renovate[bot]
- deps: update npm packages (#10685) by @renovate[bot]
- build the benchmark comparison window from the JSON lines (#10701) by @IWANABETHATGUY
- wasi: retry the Node Test step to absorb the shared-dlmalloc flake (#10699) by @hyfdev
- append benchmark results to storage as JSON lines (#10682) by @IWANABETHATGUY
- deps: update github actions (#10683) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.28.2 (#10679) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.28.1 (#10674) by @renovate[bot]

### ❤️ New Contributors

* @melbinjp made their first contribution in [#10719](#10719)
* @Freakazo made their first contribution in [#10591](#10591)
* @marcoroth made their first contribution in [#10655](#10655)

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

3 participants

Sponsor
SponsoredKunjungi sekarang
Promo