feat(core): watch loaded config files for restart#8149
Conversation
📝 WalkthroughWalkthroughConfiguration file and dependency watching is now derived by restart handling from the loaded configuration context instead of being registered during CLI config loading. An end-to-end test verifies that changing an imported configuration dependency emits a dev-server restart event. English and Chinese documentation describe configuration watching, restart behavior, and the JavaScript API requirements. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying rsbuild with
|
| Latest commit: |
16bd939
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b62eebcb.rsbuild-v2.pages.dev |
| Branch Preview URL: | https://chenjiahan-watch-loaded-conf.rsbuild-v2.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/cases/javascript-api/restart-config-files/index.test.ts (1)
32-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWriting to a file repeatedly during polling can cause excessive disk I/O and watcher events.
Because
expect.pollevaluates the callback frequently (often every ~100ms), writing toconfigDependencyon every iteration untilrestartContextis set can trigger many file system events. This might queue multiple restart requests and cause flaky test behavior or excessive disk I/O.Consider writing the file once after a brief delay, or using a less aggressive retry mechanism to trigger the watcher.
♻️ Proposed refactor (delay before write)
- try { - await expect - .poll( - () => { - if (!restartContext) { - fs.writeFileSync(configDependency, `${originalConfig}\n// ${++version}\n`); - } - return restartContext; - }, - { timeout: 5_000 }, - ) - .toEqual({ - action: 'dev', - filePath: configDependency, - }); - } finally { + try { + // Wait for watchers to initialize in the background, then trigger change + setTimeout(() => { + fs.writeFileSync(configDependency, `${originalConfig}\n// updated\n`); + }, 500); + + await expect + .poll(() => restartContext, { timeout: 5_000 }) + .toEqual({ + action: 'dev', + filePath: configDependency, + }); + } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/cases/javascript-api/restart-config-files/index.test.ts` around lines 32 - 40, Update the expect.poll callback around restartContext so it does not rewrite configDependency on every polling iteration; trigger the configuration change once after a brief delay or otherwise guard the write to prevent repeated filesystem events, while preserving the existing restartContext polling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/cases/javascript-api/restart-config-files/index.test.ts`:
- Around line 32-40: Update the expect.poll callback around restartContext so it
does not rewrite configDependency on every polling iteration; trigger the
configuration change once after a brief delay or otherwise guard the write to
prevent repeated filesystem events, while preserving the existing restartContext
polling behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88de1bdf-99f5-427a-b574-002e38a23885
📒 Files selected for processing (12)
e2e/cases/javascript-api/restart-config-files/index.test.tse2e/cases/javascript-api/restart-config-files/rsbuild.config.mjse2e/cases/javascript-api/restart-config-files/sharedConfig.mjse2e/cases/javascript-api/restart-config-files/src/index.jspackages/core/src/cli/init.tspackages/core/src/restart.tswebsite/docs/en/api/javascript-api/core.mdxwebsite/docs/en/config/dev/watch-files.mdxwebsite/docs/en/guide/configuration/rsbuild.mdxwebsite/docs/zh/api/javascript-api/core.mdxwebsite/docs/zh/config/dev/watch-files.mdxwebsite/docs/zh/guide/configuration/rsbuild.mdx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16bd9391d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/cases/javascript-api/dev-server-restart/index.test.ts (1)
41-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThrottle rapid file writes inside the polling loop.
Writing to the configuration file repeatedly inside
expect.pollwithout throttling can cause the file to be updated very rapidly (e.g., every 50ms, depending on the test runner). This risks triggering multiple concurrent dev server restarts, which may lead to test flakiness, resource leaks, orEADDRINUSEerrors if the server attempts to restart in the background.Consider increasing the polling interval (e.g., using
{ interval: 500 }or{ intervals: [500] }depending on the test framework configuration) to give the server enough time to process the initial file change and emit the restart event before writing again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/cases/javascript-api/dev-server-restart/index.test.ts` around lines 41 - 49, Throttle the expect.poll loop that writes configDep by configuring a polling interval of about 500ms, while preserving the existing restartContext check and expected result. Use the polling options supported by the test framework so repeated writes allow the dev server to process each restart.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/cases/javascript-api/dev-server-restart/index.test.ts`:
- Around line 41-49: Throttle the expect.poll loop that writes configDep by
configuring a polling interval of about 500ms, while preserving the existing
restartContext check and expected result. Use the polling options supported by
the test framework so repeated writes allow the dev server to process each
restart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d94da1e-fb0c-46dc-acd4-4d366ba4222f
📒 Files selected for processing (7)
e2e/cases/javascript-api/dev-server-restart/index.test.tse2e/cases/javascript-api/dev-server-restart/rsbuild.config.mjse2e/cases/javascript-api/dev-server-restart/src/index.jswebsite/docs/en/api/javascript-api/core.mdxwebsite/docs/en/config/dev/watch-files.mdxwebsite/docs/zh/api/javascript-api/core.mdxwebsite/docs/zh/config/dev/watch-files.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- website/docs/en/api/javascript-api/core.mdx
- website/docs/zh/api/javascript-api/core.mdx
1 participant
Summary
This PR lets JavaScript API users automatically watch loaded configuration files and their imported dependencies for restart requests. It moves config file watching from the CLI into Core and adds focused end-to-end coverage and documentation for the CLI and JavaScript API behavior.
Related Links