fix(native-watcher): fix aggregate handler trigger #12203
+394
−120
Conversation
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
✅ Deploy Preview for rspack canceled.
|
github-actions
bot
added
the
team
Contributor
📦 Binary Size-limit
❌ Size increased by 1.63KB from 47.45MB to 47.46MB (⬆️0.00%) |
CodSpeed Performance ReportMerging #12203 will not alter performanceComparing Summary
|
h-a-n-a
force-pushed
the
port-watchpack-tests
branch
from
a660330 to
91c78a1
Compare
h-a-n-a
changed the title
github-actions
bot
added
the
release: bug fix
Copilot finished reviewing on behalf of
h-a-n-a
November 14, 2025 11:08
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the aggregate handler trigger mechanism in the native watcher by introducing a last_changed field to track when the last file change occurred. The aggregate handler now monitors this timestamp and triggers only after the specified timeout has elapsed since the last change, rather than triggering immediately.
Key Changes
- Introduced
last_changed: Arc<RwLock<Option<Instant>>>to track the timestamp of the last file system event - Refactored aggregate task to use a polling loop that checks elapsed time since last change
- Restructured test helpers with new macros and improved error handling
- Added comprehensive tests for single and multiple file watching scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| crates/rspack_watcher/src/executor.rs | Core logic changes: added last_changed field and refactored aggregate timeout handling from event-based to time-based polling |
| crates/rspack_watcher/tests/watcher.rs | New test cases for single file, ignored files (glob/regex), and multiple file watching scenarios |
| crates/rspack_watcher/tests/helpers/mod.rs | Added collect_events (non-blocking), tick_ms, improved error handling, and changed time precision to microseconds |
| crates/rspack_watcher/tests/helpers/macros.rs | Extracted test macros into separate module for better organization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GiveMe-A-Name
approved these changes
Nov 18, 2025
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
Pass more native watcher tests.
last_changedto indicate when the last change was made. This field is monitored by the event aggregation handler. Once timeout, the handler will be triggered, otherwise the thread will be put to sleep for the remaining duration. (This fixed the case ofshould watch multiple fileswhere aggregation handler is not triggered according to the last modification)Related links
Checklist