test(dev): expect a hot update across a circular import - #10700
Merged
Merged
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
hyfdev
force-pushed
the
test/circular-hot-update
branch
2 times, most recently
from
August 18, 2026 13:23
f9bddb6 to
20ed763
Compare
h-a-n-a
approved these changes
Aug 18, 2026
Member
Author
Merge activity
|
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
Bot
force-pushed
the
test/circular-hot-update
branch
from
August 18, 2026 14:00
20ed763 to
04eaa68
Compare
Contributor
There was a problem hiding this comment.
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
ccwhile 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.
Merged
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix dev test ci error found in #10699
The dev-server harness builds whatever
vitejs/viterolldown-canaryrebased ontomaingives 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. EveryNode Dev Server Testjob has been red since, on every branch and every Node version, includingmain.The playground
What changed upstream
Editing
c.jsmakes the HMR client walk up the importers looking for a module that accepts updates:It used to drop the whole walk the moment it looped back — including the
mainboundary it had already found — and reload the page:vitejs/vite#23259 makes it skip the path it already walked and keep the boundary it found, so the edit is applied through
mainand 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-circularpinned the old behavior: wait for that log line, then assert the page reloaded (a marker planted onwindowis gone). Neither holds now — the log is deleted, so the wait times out withTimeout 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
mainat 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.