refactor(rust/watch): simply watch logic in the binding layer#8516
Merged
graphite-app[bot] merged 1 commit intomainfrom Mar 2, 2026
Merged
Conversation
This was referenced Mar 2, 2026
Member
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies the watch mode implementation in the binding layer by aligning rolldown_watcher::Watcher with the existing DevEngine new → run → close lifecycle pattern. The key changes move the event listener from the start() method into the constructor and eliminate the state machine in BindingWatcher, replacing it with direct delegation to the core Watcher.
Changes:
- Moves the event listener/callback from
BindingWatcher::start(listener)into the constructor, and renamesstart()torun()(which no longer takes a listener argument) - Eliminates the
BindingWatcherStatestate machine in the binding layer;BindingWatcherbecomes a thin wrapper that delegates directly torolldown_watcher::Watcher - Simplifies
WatcherMsg::Closefrom a oneshot-channel reply to a fire-and-forget message, with coordinator completion tracked via a sharedfutures::future::Sharedfuture instead
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/rolldown_watcher/src/watcher.rs |
Core refactor: adds run() + shared-future wait_for_close(), removes with_multiple_bundler_configs, drops old Arc<Notify>/JoinHandle fields |
crates/rolldown_watcher/src/watcher_msg.rs |
Simplifies Close variant from Close(oneshot::Sender<()>) to plain Close |
crates/rolldown_watcher/src/watch_coordinator.rs |
Updates handle_close() to take no arguments, removes oneshot reply |
crates/rolldown_binding/src/watcher.rs |
Removes BindingWatcherState, moves listener to constructor, delegates to inner Watcher |
crates/rolldown_watcher/src/lib.rs |
Removes WatchTaskIdx from public re-exports |
crates/rolldown_watcher/Cargo.toml + Cargo.lock |
Adds futures dependency |
packages/rolldown/src/api/watch/watcher.ts |
Extracts createEventCallback as standalone function, moves process.nextTick(() => this.run()) to constructor, simplifies run() |
packages/rolldown/src/binding.d.cts |
Updates TypeScript declarations: constructor now takes listener, start() → run() |
crates/rolldown/examples/watch_new.rs |
Updates example to use new vec![config] + watcher.run() API |
meta/design/watch-mode.md |
Updates design doc to reflect new new → run → close pattern and thin-wrapper architecture |
AGENTS.md |
Replaces "traceability" guideline with "keep docs in sync" and "reference in code" guidelines |
Contributor
Benchmarks Rust
|
a73bd03 to
cb1e1fe
Compare
825ea08 to
26f3426
Compare
IWANABETHATGUY
approved these changes
Mar 2, 2026
shulaoda
approved these changes
Mar 2, 2026
cb1e1fe to
847093e
Compare
Contributor
Merge activity
|
- I cleaned up unnecessary complex logic - Move heavy logic into rust part, make binding layer only a wrapper of rust part --- ## Summary Align Watcher construction with DevEngine's `new → run → close` pattern. **Rust core (`rolldown_watcher`)** - Replace `new_pending(configs, config)` + `start(handler)` with `new(configs, handler, &config)` + `run()` - Remove `new_pending`, `with_multiple_bundler_configs`, single-config `new` convenience - Replace `Arc<Notify>` with `Shared<Future>` for `wait_for_close` — idempotent, no race condition - Simplify `WatcherMsg::Close` — remove `oneshot::Sender` payload, `close()` awaits the shared future instead - Remove `Mutex<Option<>>` wrapper on `tx` — `UnboundedSender::send` takes `&self`, no lock needed - Remove unused `WatchTaskIdx` re-export - Add `futures` dep for `Shared`/`FutureExt` **NAPI binding (`rolldown_binding`)** - Make `BindingWatcher` a thin wrapper — constructor takes both `options` and `listener`, no state machine or locking - Replace `start(listener)` with `run()` (just spawns coordinator) **JS layer (`watcher.ts`)** - Extract `createEventCallback` to standalone function, pass to `BindingWatcher` constructor - Move `process.nextTick` into constructor, make `run()` private — matches Rollup's pattern **Docs** - Update `meta/design/watch-mode.md` to reflect new API, close flow, binding-as-thin-wrapper principle - Update `AGENTS.md`: reference design docs in code, keep docs in sync with code changes 🤖 Generated with [Claude Code](https://claude.com/claude-code)
847093e to
6b868eb
Compare
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This was referenced Mar 2, 2026
shulaoda
added a commit
that referenced
this pull request
Mar 5, 2026
## [1.0.0-rc.7] - 2026-03-05 ⚡ Smarter Code Generation Defaults - DCE-only minification and smart constant inlining are now enabled by default - Produces cleaner, smaller output bundles without requiring explicit configuration 💡 LLM-Friendly Bundle Analyzer Reports - New markdown output format for the bundle analyzer plugin with bundle summaries, module graphs, dependency chains, and optimization suggestions - Optimization suggestions now also recommend using the entriesAware option when common chunks contain modules only reachable from specific entries ### 💥 BREAKING CHANGES - enable minify: 'dce-only' by default (#8465) by @IWANABETHATGUY - settings `inlineConst: { mode: 'smart', pass: 1}` by default (#8444) by @IWANABETHATGUY ### 🚀 Features - binding: add original getter to BindingMagicString (#8533) by @IWANABETHATGUY - native-magic-string: add `offset` property support (#8531) by @IWANABETHATGUY - add `output.strict` option to control `"use strict"` directive emission (#8489) by @Copilot - watch: expose `watcher.compareContentsForPolling` (#8526) by @hyf0 - watch: use new watcher to support watch mode (#8475) by @hyf0 - rust/watch: handle bulk-change (#8466) by @hyf0 - add LLM-friendly markdown output format to bundle analyzer plugin (#8242) by @IWANABETHATGUY ### 🐛 Bug Fixes - expose `plugins` on `NormalizedInputOptions` for `buildStart` hook (#8521) by @Copilot - only uppercase facade symbols in JSX preserve mode (#8519) by @IWANABETHATGUY - binding: export BindingResult in generated dts header (#8537) by @minsoo-web - pre-resolve paths option to avoid `invoke_sync` deadlock (#8518) by @IWANABETHATGUY - remove debug-only jsx_preset and UntranspiledSyntaxError (#8511) by @IWANABETHATGUY - apply `topLevelVar` to exported `const`/`let` declarations (#8507) by @IWANABETHATGUY - rolldown_plugin_vite_web_worker_post: avoid replacing `new.target` (#8488) by @sapphi-red - update copyright year to 2026 (#8486) by @maciekzygmunt ### 🚜 Refactor - rust: use Oxc's SymbolFlags::ConstVariable instead of custom IsConst flag (#8543) by @Dunqing - rust: remove FacadeScoping, use Scoping::create_symbol for facade symbols (#8540) by @Dunqing - rust/watch: remove hacky `reset_closed_for_watch_mode` (#8530) by @hyf0 - binding: return &str instead of String in filename() getter (#8534) by @IWANABETHATGUY - rust: remove old watch mode implementation (#8525) by @hyf0 - rust/watch: simply watch logic in the binding layer (#8516) by @hyf0 - rust/watch: tweak struct/function names (#8464) by @hyf0 ### 📚 Documentation - explain how external modules work in rolldown (#8457) by @sapphi-red - add some diagrams using graphviz (#8499) by @sapphi-red - use `vitepress-plugin-graphviz` (#8498) by @sapphi-red - list s390x/ppc64le prebuilt binaries (#8495) by @crusty-voidzero - fix error type for `RolldownBuild.generate` and others (#8490) by @sapphi-red ### ⚡ Performance - string_wizard: reduce allocations and add ASCII fast paths (#8541) by @IWANABETHATGUY - use IndexBitSet to replace IndexVec<XXXIdx, bool> for module/stmt inclusion tracking (#8503) by @IWANABETHATGUY - plugin: use IndexBitSet to optimize skipped plugins checking (#8497) by @ShroXd - rust/tla: skip compute_tla if there is no module use TLA (#8487) by @ShroXd ### 🧪 Testing - node/watch: make watch tests run in concurrent and retry-able (#8512) by @hyf0 - add test case for static flag tree-shaking (#8476) by @IWANABETHATGUY - migrate post-banner sourcemap-with-shebang to Rust (#8477) by @Copilot ### ⚙️ Miscellaneous Tasks - vscode: `formatOnSave` for markdown files using oxc formatter (#8536) by @minsoo-web - deps: update test262 submodule for tests (#8528) by @sapphi-red - remove `retry` workaround from output paths test fixtures (#8520) by @Copilot - docs: add Shuyuan Wang (h-a-n-a) and remove from acknowledgements (#8509) by @Copilot - consolidate top_level_var test cases using configVariants (#8508) by @IWANABETHATGUY - add s390x and ppc64le linux gnu targets (#8493) by @Brooooooklyn ###◀️ Revert - fix(rolldown): increase tokio blocking threads size for watch mode (#8517) by @hyf0 ### ❤️ New Contributors * @minsoo-web made their first contribution in [#8536](#8536) * @crusty-voidzero made their first contribution in [#8495](#8495) * @maciekzygmunt made their first contribution in [#8486](#8486) 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.
Summary
Align Watcher construction with DevEngine's
new → run → closepattern.Rust core (
rolldown_watcher)new_pending(configs, config)+start(handler)withnew(configs, handler, &config)+run()new_pending,with_multiple_bundler_configs, single-confignewconvenienceArc<Notify>withShared<Future>forwait_for_close— idempotent, no race conditionWatcherMsg::Close— removeoneshot::Senderpayload,close()awaits the shared future insteadMutex<Option<>>wrapper ontx—UnboundedSender::sendtakes&self, no lock neededWatchTaskIdxre-exportfuturesdep forShared/FutureExtNAPI binding (
rolldown_binding)BindingWatchera thin wrapper — constructor takes bothoptionsandlistener, no state machine or lockingstart(listener)withrun()(just spawns coordinator)JS layer (
watcher.ts)createEventCallbackto standalone function, pass toBindingWatcherconstructorprocess.nextTickinto constructor, makerun()private — matches Rollup's patternDocs
meta/design/watch-mode.mdto reflect new API, close flow, binding-as-thin-wrapper principleAGENTS.md: reference design docs in code, keep docs in sync with code changes🤖 Generated with Claude Code