feat(rspeedy): apply dev.hmr and dev.liveReload config to Rsbuild config#1882
Conversation
🦋 Changeset detectedLatest commit: 8bb39c3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReads HMR and live-reload settings from environment-specific config (defaulting to true), adds defaults for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/dull-kiwis-like.md (1)
5-5: Consider expanding the changeset description.The current description is concise but could be more informative for users reviewing release notes. Consider adding context about what this fixes or why it matters.
For example:
-Should apply `dev.hmr` and `dev.liveReload` to Rsbuild config. +Fix inconsistency where `dev.hmr` and `dev.liveReload` config values differ when accessed via the exposed user config versus environment config in Rsbuild plugins. These settings now default to `true` in the Rsbuild config, ensuring consistent behavior across plugin access methods.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/dull-kiwis-like.md(1 hunks)packages/rspeedy/core/src/config/rsbuild/index.ts(1 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/dull-kiwis-like.md
🧠 Learnings (1)
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/dull-kiwis-like.md
🧬 Code graph analysis (1)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: zizmor
🔇 Additional comments (7)
packages/rspeedy/core/src/config/rsbuild/index.ts (1)
27-28: LGTM!The addition of default values for
hmrandliveReloadcorrectly addresses the inconsistency described in the PR objectives. Defaulting both totruealigns with standard development server expectations and ensures these values are consistently available in the Rsbuild config for other plugins to read fromenvironment.config.packages/rspeedy/core/test/plugins/dev.plugin.test.ts (6)
433-433: LGTM!Good addition to verify the default
hmrvalue is accessible via the publicgetRsbuildConfig()API, addressing the inconsistency noted in the PR objectives.Also applies to: 439-439
450-450: LGTM!Correctly verifies that explicitly setting
hmr: falseis reflected in the public Rsbuild config.Also applies to: 456-456
467-467: LGTM!Properly tests explicit
hmr: trueconfiguration through the public API.Also applies to: 473-473
480-480: LGTM!Good coverage for the default
liveReloadbehavior viagetRsbuildConfig().Also applies to: 486-486
497-497: LGTM!Correctly validates that
liveReload: falseis accessible through the public config API.Also applies to: 503-503
514-514: LGTM!Appropriate test coverage for explicit
liveReload: trueconfiguration.Also applies to: 520-520
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
dev.hmr and dev.liveReload config to Rsbuild configdev.hmr and dev.liveReload config to Rsbuild config
dev.hmr and dev.liveReload config to Rsbuild configdev.hmr and dev.liveReload config to Rsbuild config
React Example#5879 Bundle Size — 237.56KiB (0%).8bb39c3(current) vs dd7c14a main#5878(baseline) Bundle metrics
|
| Current #5879 |
Baseline #5878 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
166 |
166 |
|
68 |
68 |
|
46.82% |
46.82% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5879 |
Baseline #5878 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.8KiB |
91.8KiB |
Bundle analysis report Branch luhc228:feat/apply-hmr-liveReloa... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5874 Bundle Size — 364.46KiB (0%).8bb39c3(current) vs dd7c14a main#5873(baseline) Bundle metrics
Bundle size by type
|
| Current #5874 |
Baseline #5873 |
|
|---|---|---|
238.66KiB |
238.66KiB |
|
93.8KiB |
93.8KiB |
|
32KiB |
32KiB |
Bundle analysis report Branch luhc228:feat/apply-hmr-liveReloa... Project dashboard
Generated by RelativeCI Documentation Report issue
e14b458 to
0911c3f
Compare
CodSpeed Performance ReportMerging #1882 will degrade performances by 6.35%Comparing Summary
Benchmarks breakdown
Footnotes
|
0911c3f to
06899f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
489-506: Standardize test naming for consistency.The test names have minor inconsistencies:
- Line 455 uses
"environment.dev.hmr: false"(with dot)- Line 489 uses
"environment dev.hmr: true"(without dot)- Line 534 uses
"environment dev.liveReload: false"(without dot)- Line 568 uses
"environments dev.liveReload: true"(plural, without dot)The test logic is correct, but for consistency, consider standardizing the naming convention. Either use
"environment.dev"with a dot throughout, or"environment dev"without a dot, and keep "environment" singular.Apply this diff to standardize the naming:
- test('environment dev.hmr: true', async () => { + test('environment.dev.hmr: true', async () => { const rsbuild = await createStubRspeedy({ environments: { lynx: { dev: { hmr: true, }, }, }, }) const config = await rsbuild.unwrapConfig() expect(config.resolve?.alias).toHaveProperty( '@lynx-js/webpack-dev-transport/client', expect.stringContaining('hot=true'), ) }) - test('environment dev.liveReload: false', async () => { + test('environment.dev.liveReload: false', async () => { const rsbuild = await createStubRspeedy({ environments: { lynx: { dev: { liveReload: false, }, }, }, }) const config = await rsbuild.unwrapConfig() expect(config.resolve?.alias).toHaveProperty( '@lynx-js/webpack-dev-transport/client', expect.stringContaining('live-reload=false'), ) }) - test('environments dev.liveReload: true', async () => { + test('environment.dev.liveReload: true', async () => { const rsbuild = await createStubRspeedy({ environments: { lynx: { dev: { liveReload: true, }, }, }, }) const config = await rsbuild.unwrapConfig() expect(config.resolve?.alias).toHaveProperty( '@lynx-js/webpack-dev-transport/client', expect.stringContaining('live-reload=true'), ) })Also applies to: 534-551, 568-585
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/dull-kiwis-like.md(1 hunks)packages/rspeedy/core/src/config/rsbuild/index.ts(1 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(1 hunks)packages/rspeedy/core/test/config/rsbuild.test.ts(1 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rspeedy/core/src/plugins/dev.plugin.ts
- packages/rspeedy/core/src/config/rsbuild/index.ts
- .changeset/dull-kiwis-like.md
🧰 Additional context used
🧬 Code graph analysis (1)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
🔇 Additional comments (2)
packages/rspeedy/core/test/config/rsbuild.test.ts (1)
19-21: LGTM!The snapshot correctly reflects the new default values for
hmrandliveReloadwhen the dev config is undefined. This aligns with the PR objective of ensuring these flags are properly applied to the rsbuild configuration.packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
455-472: LGTM!The test correctly verifies that when
hmris set tofalsein environment-specific config, the transport client alias reflects this withhot=false. This aligns with the PR objective of ensuring environment config is properly propagated.
| hmr: config.dev?.hmr ?? true, | ||
| liveReload: config.dev?.liveReload ?? true, |
There was a problem hiding this comment.
nitpick: we should sort in alphabetical order
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/web-core@0.18.0 ### Minor Changes - fix: ([#1837](#1837)) 1. `LynxView.updateData()` cannot trigger `dataProcessor`. 2. **This is a break change:** The second parameter of `LynxView.updateData()` has been changed from `UpdateDataType` to `string`, which is the `processorName` (default is `default` which will use `defaultDataProcessor`). This change is to better align with Native. The current complete type is as follows: ```ts LynxView.updateData(data: Cloneable, processorName?: string | undefined, callback?: (() => void) | undefined): void ``` ### Patch Changes - Updated dependencies \[[`77397fd`](77397fd), [`7d90ed5`](7d90ed5)]: - @lynx-js/web-worker-runtime@0.18.0 - @lynx-js/web-constants@0.18.0 - @lynx-js/web-mainthread-apis@0.18.0 - @lynx-js/web-worker-rpc@0.18.0 ## @lynx-js/react@0.114.2 ### Patch Changes - fix: main thread functions created during the initial render cannot correctly call `runOnBackground()` after hydration ([#1878](#1878)) ## @lynx-js/rspeedy@0.11.6 ### Patch Changes - Should apply `dev.hmr` and `dev.liveReload` to Rsbuild config. ([#1882](#1882)) - Support CLI flag `--root` to specify the root of the project. ([#1836](#1836)) ## @lynx-js/react-rsbuild-plugin@0.11.2 ### Patch Changes - Fix using wrong version of `@lynx-js/react/refresh`. ([#1756](#1756)) - Updated dependencies \[]: - @lynx-js/react-alias-rsbuild-plugin@0.11.2 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.4 - @lynx-js/react-webpack-plugin@0.7.1 ## @lynx-js/web-constants@0.18.0 ### Patch Changes - fix: ([#1837](#1837)) 1. `LynxView.updateData()` cannot trigger `dataProcessor`. 2. **This is a break change:** The second parameter of `LynxView.updateData()` has been changed from `UpdateDataType` to `string`, which is the `processorName` (default is `default` which will use `defaultDataProcessor`). This change is to better align with Native. The current complete type is as follows: ```ts LynxView.updateData(data: Cloneable, processorName?: string | undefined, callback?: (() => void) | undefined): void ``` - Updated dependencies \[]: - @lynx-js/web-worker-rpc@0.18.0 ## @lynx-js/web-elements@0.8.8 ### Patch Changes - fix: ([#1837](#1837)) 1. `LynxView.updateData()` cannot trigger `dataProcessor`. 2. **This is a break change:** The second parameter of `LynxView.updateData()` has been changed from `UpdateDataType` to `string`, which is the `processorName` (default is `default` which will use `defaultDataProcessor`). This change is to better align with Native. The current complete type is as follows: ```ts LynxView.updateData(data: Cloneable, processorName?: string | undefined, callback?: (() => void) | undefined): void ``` - Updated dependencies \[]: - @lynx-js/web-elements-template@0.8.8 ## @lynx-js/web-explorer@0.0.10 ### Patch Changes - chore: update `@lynx-js/lynx-core` to `0.1.3`, `@lynx-js/web-core` to `0.17.1`. ([#1839](#1839)) ## @lynx-js/web-mainthread-apis@0.18.0 ### Patch Changes - fix: ([#1837](#1837)) 1. `LynxView.updateData()` cannot trigger `dataProcessor`. 2. **This is a break change:** The second parameter of `LynxView.updateData()` has been changed from `UpdateDataType` to `string`, which is the `processorName` (default is `default` which will use `defaultDataProcessor`). This change is to better align with Native. The current complete type is as follows: ```ts LynxView.updateData(data: Cloneable, processorName?: string | undefined, callback?: (() => void) | undefined): void ``` - feat: mouse event output structures remain aligned ([#1820](#1820)) - Updated dependencies \[[`77397fd`](77397fd)]: - @lynx-js/web-constants@0.18.0 - @lynx-js/web-style-transformer@0.18.0 ## @lynx-js/web-worker-runtime@0.18.0 ### Patch Changes - fix: ([#1837](#1837)) 1. `LynxView.updateData()` cannot trigger `dataProcessor`. 2. **This is a break change:** The second parameter of `LynxView.updateData()` has been changed from `UpdateDataType` to `string`, which is the `processorName` (default is `default` which will use `defaultDataProcessor`). This change is to better align with Native. The current complete type is as follows: ```ts LynxView.updateData(data: Cloneable, processorName?: string | undefined, callback?: (() => void) | undefined): void ``` - Updated dependencies \[[`77397fd`](77397fd), [`7d90ed5`](7d90ed5)]: - @lynx-js/web-constants@0.18.0 - @lynx-js/web-mainthread-apis@0.18.0 - @lynx-js/web-worker-rpc@0.18.0 ## create-rspeedy@0.11.6 ## @lynx-js/react-alias-rsbuild-plugin@0.11.2 ## upgrade-rspeedy@0.11.6 ## @lynx-js/web-core-server@0.18.0 ## @lynx-js/web-elements-template@0.8.8 ## @lynx-js/web-rsbuild-server-middleware@0.18.0 ## @lynx-js/web-style-transformer@0.18.0 ## @lynx-js/web-worker-rpc@0.18.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Related PR: #458
If we set
dev.hmrordev.liveReloadtofalseinlynx.config.js:Now we will get different value of
dev.hmranddev.liveReloadin rsbuild plugin:We can also set these two config in
environmentconfig and above can't read the real value by the first way above:I think Rspeedy should apply the two configs to rsbuild config and other plugins should read the config from
environment.configfirst.Summary by CodeRabbit
Checklist