WIP Feat/native watcher on emit #11581
Conversation
✅ Deploy Preview for rspack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
added
the
team
There was a problem hiding this comment.
Pull Request Overview
This PR adds event emitter functionality to the native file watcher system, enabling watch file systems to emit, listen for, and handle file system events through standard event patterns.
- Extended the
WatchFileSysteminterface with event emitter methods (on,once,emit) - Modified the native watcher to emit "change" and "remove" events with proper event data
- Updated Rust event handlers to include modification time information and renamed delete operations to remove
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rspack/src/util/fs.ts | Adds optional event emitter interface methods to WatchFileSystem |
| packages/rspack/src/node/NodeWatchFileSystem.ts | Implements event emitter methods for Node.js-based file watching |
| packages/rspack/src/NativeWatchFileSystem.ts | Implements event emitter methods for native file watching with internal EventEmitter |
| packages/rspack/etc/core.api.md | Updates public API documentation to include new event emitter methods |
| crates/rspack_fs/src/watcher/mod.rs | Updates Rust trait to include mtime parameter and renames delete to remove |
| crates/rspack_fs/src/watcher/executor.rs | Updates event handler calls to match new trait signature |
| crates/rspack_binding_api/src/native_watcher.rs | Modifies JS binding to pass event type and mtime data to callbacks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| (type: "change" | "remove", fileName: string, mtime?: number) => { | ||
| // FIXME: napi-rs will pass all arguments as array | ||
| // @ts-ignore | ||
| console.log("NativeWatchFileSystem event", type, fileName, mtime); |
There was a problem hiding this comment.
Debug console.log statement should be removed before merging to production. Consider using a proper logging mechanism or removing this line entirely.
| console.log("NativeWatchFileSystem event", type, fileName, mtime); |
Copilot uses AI. Check for mistakes.
| @@ -1,9 +1,11 @@ | |||
| use std::{ | |||
| alloc::System, | |||
There was a problem hiding this comment.
The alloc::System import appears to be unused. This import should be removed to avoid clutter.
| alloc::System, |
Copilot uses AI. Check for mistakes.
| }; | ||
|
|
||
| use napi::bindgen_prelude::*; | ||
| use napi::{JsUndefined, bindgen_prelude::*}; |
There was a problem hiding this comment.
The JsUndefined import appears to be unused. This import should be removed to avoid clutter.
| use napi::{JsUndefined, bindgen_prelude::*}; | |
| use napi::bindgen_prelude::*; |
Copilot uses AI. Check for mistakes.
GiveMe-A-Name
changed the title
📦 Binary Size-limit
❌ Size increased by 3.50KB from 47.28MB to 47.28MB (⬆️0.01%) |
Summary
Related links
Checklist