feat(qrcode): support get entry from api exposed from rspeedy.env.entries#2551
Conversation
🦋 Changeset detectedLatest commit: 55f78e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional Changesrspeedy entries + QR-code plugin
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
4dc5308 to
c5bccc2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rspeedy/plugin-qrcode/test/index.test.ts (1)
336-383: ⚡ Quick winStrengthen the new exposed-entries test with an explicit URL assertion.
The new case verifies invocation count, but not the generated URL. Adding
toBeCalledWith(...)will lock the contract this feature introduces.✅ Suggested assertion
await server.waitDevCompileDone() expect(renderUnicodeCompact).toBeCalledTimes(1) + expect(renderUnicodeCompact).toBeCalledWith( + 'http://example.com/foo/main.lynx.bundle', + ) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-qrcode/test/index.test.ts` around lines 336 - 383, Update the test 'print qrcode with exposed custom environment entries' to assert the actual generated URL passed into renderUnicodeCompact: after server.waitDevCompileDone(), add an expectation that renderUnicodeCompact was called with the QR payload containing the full URL built from the rsbuildConfig.dev.assetPrefix ('http://example.com/foo/') combined with the exposed entry path ('hello-world' from the entry variable) so the test checks renderUnicodeCompact (the mocked function) received the QR data that includes the exact expected URL produced by pluginQRCode.packages/rspeedy/plugin-qrcode/src/index.ts (1)
134-140: Use a dedicated exposed type forrspeedy.env.entriesinstead ofExposedAPI.The symbol exposes only
{ entries?: RsbuildEntry }, but the current type parameterExposedAPIdeclares a broader shape including other properties not exposed here. This violates strict TypeScript type safety by allowing potential access to non-existent properties.Define a dedicated type for this exposure:
♻️ Suggested change
+type EnvEntriesExposedAPI = { entries?: RsbuildEntry } + function getEntries( environments: Record<string, EnvironmentContext> | undefined, ) { // biome-ignore lint/correctness/useHookAtTopLevel: not react hooks - return api.useExposed<ExposedAPI>(Symbol.for('rspeedy.env.entries')) + return api.useExposed<EnvEntriesExposedAPI>(Symbol.for('rspeedy.env.entries')) ?.entries ?? environments?.['lynx']?.entry }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-qrcode/src/index.ts` around lines 134 - 140, The code uses api.useExposed<ExposedAPI>(Symbol.for('rspeedy.env.entries')) in getEntries which is too broad; define a narrow exposed type (e.g., EnvEntriesExposed with a single optional property entries?: RsbuildEntry) and use that type as the generic parameter instead of ExposedAPI so TypeScript only permits the actual shape exposed by Symbol.for('rspeedy.env.entries'); update the getEntries call to api.useExposed<EnvEntriesExposed>(Symbol.for('rspeedy.env.entries')) and add the new type definition/import where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rspeedy/plugin-qrcode/src/index.ts`:
- Around line 134-140: The code uses
api.useExposed<ExposedAPI>(Symbol.for('rspeedy.env.entries')) in getEntries
which is too broad; define a narrow exposed type (e.g., EnvEntriesExposed with a
single optional property entries?: RsbuildEntry) and use that type as the
generic parameter instead of ExposedAPI so TypeScript only permits the actual
shape exposed by Symbol.for('rspeedy.env.entries'); update the getEntries call
to api.useExposed<EnvEntriesExposed>(Symbol.for('rspeedy.env.entries')) and add
the new type definition/import where appropriate.
In `@packages/rspeedy/plugin-qrcode/test/index.test.ts`:
- Around line 336-383: Update the test 'print qrcode with exposed custom
environment entries' to assert the actual generated URL passed into
renderUnicodeCompact: after server.waitDevCompileDone(), add an expectation that
renderUnicodeCompact was called with the QR payload containing the full URL
built from the rsbuildConfig.dev.assetPrefix ('http://example.com/foo/')
combined with the exposed entry path ('hello-world' from the entry variable) so
the test checks renderUnicodeCompact (the mocked function) received the QR data
that includes the exact expected URL produced by pluginQRCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2ad2bc5-a22e-4ec7-931c-0091628c78ff
📒 Files selected for processing (5)
.changeset/wet-humans-teach.mdpackages/rspeedy/core/src/api.tspackages/rspeedy/plugin-qrcode/src/index.tspackages/rspeedy/plugin-qrcode/test/index.test.tspackages/rspeedy/plugin-qrcode/test/preview.test.ts
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
c5bccc2 to
1b6eaf2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
1b6eaf2 to
45a5eb6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rspeedy/core/src/api.ts (1)
57-60: ⚡ Quick winPrefer a dedicated exposed type for
rspeedy.env.entriesinstead of extendingExposedAPI.
ExposedAPIis documented forSymbol.for('rspeedy.api'), whileentriesis consumed fromSymbol.for('rspeedy.env.entries'). Splitting these keeps the public contract clearer and avoids symbol/payload coupling.♻️ Proposed refactor
export interface ExposedAPI { @@ - /** - * Used for plugin qrcode get entry points from self-defined environments rather than default lynx environment. - */ - entries?: RsbuildEntry } + +export interface EnvEntriesExposedAPI { + /** + * Entry points exposed via Symbol.for('rspeedy.env.entries'). + */ + entries?: RsbuildEntry +}And in
packages/rspeedy/plugin-qrcode/src/index.ts, useEnvEntriesExposedAPIforapi.useExposed(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/core/src/api.ts` around lines 57 - 60, Split the `entries?: RsbuildEntry` field out of `ExposedAPI` into a dedicated exported interface (e.g., `EnvEntriesExposedAPI`) so `rspeedy.env.entries` has its own public type; update the definition to export `EnvEntriesExposedAPI` with `entries?: RsbuildEntry`, remove `entries` from `ExposedAPI`, and change usages such as `api.useExposed(...)` in `packages/rspeedy/plugin-qrcode/src/index.ts` to refer to `EnvEntriesExposedAPI` instead of `ExposedAPI`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rspeedy/core/src/api.ts`:
- Around line 57-60: Split the `entries?: RsbuildEntry` field out of
`ExposedAPI` into a dedicated exported interface (e.g., `EnvEntriesExposedAPI`)
so `rspeedy.env.entries` has its own public type; update the definition to
export `EnvEntriesExposedAPI` with `entries?: RsbuildEntry`, remove `entries`
from `ExposedAPI`, and change usages such as `api.useExposed(...)` in
`packages/rspeedy/plugin-qrcode/src/index.ts` to refer to `EnvEntriesExposedAPI`
instead of `ExposedAPI`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5123b158-54e0-41a1-9d72-62b62cbdbc11
📒 Files selected for processing (6)
.changeset/wet-humans-teach.mdpackages/rspeedy/core/etc/rspeedy.api.mdpackages/rspeedy/core/src/api.tspackages/rspeedy/plugin-qrcode/src/index.tspackages/rspeedy/plugin-qrcode/test/index.test.tspackages/rspeedy/plugin-qrcode/test/preview.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/rspeedy/core/etc/rspeedy.api.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/wet-humans-teach.md
- packages/rspeedy/plugin-qrcode/test/index.test.ts
- packages/rspeedy/plugin-qrcode/test/preview.test.ts
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
45a5eb6 to
862c452
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rspeedy/core/src/api.ts (1)
57-60: ⚡ Quick winSplit symbol-specific exposed types instead of extending
ExposedAPILine 57 introduces
entriesonExposedAPI, but this field is read viaSymbol.for('rspeedy.env.entries')(notSymbol.for('rspeedy.api')). Keeping both contracts in one interface dilutes type safety for both symbols.Suggested refactor
export interface ExposedAPI { @@ - /** - * Used for plugin qrcode get entry points from self-defined environments rather than default lynx environment. - */ - entries?: RsbuildEntry } + +export interface ExposedEnvEntriesAPI { + /** + * Entry points exposed for environment-aware consumers (e.g. qrcode plugin). + */ + entries?: RsbuildEntry +}And in
packages/rspeedy/plugin-qrcode/src/index.ts:api.useExposed<ExposedEnvEntriesAPI>(Symbol.for('rspeedy.env.entries'))?.entries🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/core/src/api.ts` around lines 57 - 60, The ExposedAPI interface incorrectly includes the symbol-specific field `entries` which is actually provided under a different symbol; split out a dedicated interface (e.g., ExposedEnvEntriesAPI) that declares `entries?: RsbuildEntry` and stop adding `entries` to ExposedAPI; update consumers to read via the env symbol (for example in packages/rspeedy/plugin-qrcode/src/index.ts call api.useExposed<ExposedEnvEntriesAPI>(Symbol.for('rspeedy.env.entries'))?.entries) so typing matches the symbol used to publish the data (refer to ExposedAPI, entries, Symbol.for('rspeedy.env.entries'), ExposedEnvEntriesAPI and the plugin-qrcode usage).packages/rspeedy/plugin-qrcode/test/preview.test.ts (1)
291-325: ⚡ Quick winAlso assert no
exitcall in the empty-entries caseThis scenario already checks
renderUnicodeCompactis skipped; adding anexitassertion would lock in the same side-effect expectation as the no-lynx test and avoid silent regressions.Suggested test addition
const { server } = await rsbuild.preview({ checkDistDir: false }) expect(renderUnicodeCompact).not.toBeCalled() await server.close() + expect(exit).not.toBeCalled() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/plugin-qrcode/test/preview.test.ts` around lines 291 - 325, Add an assertion that process.exit is not invoked in the "preview with empty exposed entries" test: spy/mocking of process.exit before calling rsbuild.preview (in the same test where renderUnicodeCompact is asserted not to be called), run the preview and then assert the mocked process.exit was not called (and restore the spy afterward); reference the existing test variables/functions like createRsbuild, rsbuild.preview, renderUnicodeCompact, and server.close to locate where to add the spy/assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rspeedy/core/src/api.ts`:
- Around line 57-60: The ExposedAPI interface incorrectly includes the
symbol-specific field `entries` which is actually provided under a different
symbol; split out a dedicated interface (e.g., ExposedEnvEntriesAPI) that
declares `entries?: RsbuildEntry` and stop adding `entries` to ExposedAPI;
update consumers to read via the env symbol (for example in
packages/rspeedy/plugin-qrcode/src/index.ts call
api.useExposed<ExposedEnvEntriesAPI>(Symbol.for('rspeedy.env.entries'))?.entries)
so typing matches the symbol used to publish the data (refer to ExposedAPI,
entries, Symbol.for('rspeedy.env.entries'), ExposedEnvEntriesAPI and the
plugin-qrcode usage).
In `@packages/rspeedy/plugin-qrcode/test/preview.test.ts`:
- Around line 291-325: Add an assertion that process.exit is not invoked in the
"preview with empty exposed entries" test: spy/mocking of process.exit before
calling rsbuild.preview (in the same test where renderUnicodeCompact is asserted
not to be called), run the preview and then assert the mocked process.exit was
not called (and restore the spy afterward); reference the existing test
variables/functions like createRsbuild, rsbuild.preview, renderUnicodeCompact,
and server.close to locate where to add the spy/assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dcd7ae6c-ea29-4484-9522-26d6947c9203
📒 Files selected for processing (6)
.changeset/wet-humans-teach.mdpackages/rspeedy/core/etc/rspeedy.api.mdpackages/rspeedy/core/src/api.tspackages/rspeedy/plugin-qrcode/src/index.tspackages/rspeedy/plugin-qrcode/test/index.test.tspackages/rspeedy/plugin-qrcode/test/preview.test.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/wet-humans-teach.md
- packages/rspeedy/core/etc/rspeedy.api.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rspeedy/plugin-qrcode/test/index.test.ts
- packages/rspeedy/plugin-qrcode/src/index.ts
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
862c452 to
afba154
Compare
Merging this PR will degrade performance by 16.12%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 008-many-use-state-destroyBackground |
8 ms | 9.5 ms | -16.12% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing spencerHT:feat/qrcode-entry (55f78e5) with main (f98f7a1)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React Example with Element Template#417 Bundle Size — 197.79KiB (0%).55f78e5(current) vs f98f7a1 main#409(baseline) Bundle metrics
|
| Current #417 |
Baseline #409 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
 Modules |
81(+2.53%) |
79 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.29%(-0.1%) |
40.33% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #417 |
Baseline #409 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.03KiB |
52.03KiB |
Bundle analysis report Branch spencerHT:feat/qrcode-entry Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8150 Bundle Size — 236.51KiB (0%).55f78e5(current) vs f98f7a1 main#8142(baseline) Bundle metrics
|
| Current #8150 |
Baseline #8142 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
197 |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.87% |
44.87% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8150 |
Baseline #8142 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.75KiB |
90.75KiB |
Bundle analysis report Branch spencerHT:feat/qrcode-entry Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1283 Bundle Size — 207.46KiB (0%).55f78e5(current) vs f98f7a1 main#1275(baseline) Bundle metrics
|
| Current #1283 |
Baseline #1275 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
192 |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.38% |
44.38% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1283 |
Baseline #1275 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.23KiB |
96.23KiB |
Bundle analysis report Branch spencerHT:feat/qrcode-entry Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9725 Bundle Size — 901.38KiB (0%).55f78e5(current) vs f98f7a1 main#9717(baseline) Bundle metrics
|
| Current #9725 |
Baseline #9717 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
230 |
230 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.22%(+0.04%) |
27.21% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9725 |
Baseline #9717 |
|
|---|---|---|
| JS |
497.1KiB |
497.1KiB |
| Other |
402.06KiB |
402.06KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch spencerHT:feat/qrcode-entry Project dashboard
Generated by RelativeCI Documentation Report issue
React External#1265 Bundle Size — 693.04KiB (0%).55f78e5(current) vs f98f7a1 main#1257(baseline) Bundle metrics
|
| Current #1265 |
Baseline #1257 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch spencerHT:feat/qrcode-entry Project dashboard
Generated by RelativeCI Documentation Report issue
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
afba154 to
20611bd
Compare
spencerHT
force-pushed
the
feat/qrcode-entry
branch
from
20611bd to
55f78e5
Compare
3 participants
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
Checklist