feat: enhance QR code plugin with web environment support and middleware integration #1816
Conversation
🦋 Changeset detectedLatest commit: 4ed1eae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
PupilTong
changed the title
📝 WalkthroughWalkthroughAdds web-platform preview support to the rspeedy QRCode plugin: generates separate Lynx and Web dev URLs, integrates web virtual-files middleware, updates shortcuts and QR rendering to handle dual URLs, updates tests, example config, package deps, and TypeScript project references. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
🧰 Additional context used🧬 Code graph analysis (2)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
⏰ 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). (3)
🔇 Additional comments (7)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (16)
packages/web-platform/web-rsbuild-server-middleware/turbo.json (1)
1-16: Broaden inputs to rebuild on config changes.Include common config files so build re-runs when they change.
Apply:
- "inputs": [ - "src/**" - ], + "inputs": [ + "src/**", + "package.json", + "rslib.config.ts", + "rsbuild.config.ts", + "tsconfig.json", + "vitest.config.ts" + ],packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
102-121: Tighten assertions to ensure alias is absent..not.toHaveProperty with an expected value can pass even if the property exists with a different value. Check absence explicitly.
Apply:
- expect(config.resolve?.alias).not.toHaveProperty( - '@rspack/core/hot/dev-server', - expect.stringContaining('hot/dev-server.js'.replaceAll('/', path.sep)), - ) - expect(config.resolve?.alias).not.toHaveProperty( - '@lynx-js/webpack-dev-transport/client', - expect.stringContaining( - 'packages/webpack/webpack-dev-transport'.replaceAll('/', path.sep), - ), - ) + expect(config.resolve?.alias?.['@rspack/core/hot/dev-server']).toBeUndefined() + expect(config.resolve?.alias?.['@lynx-js/webpack-dev-transport/client']).toBeUndefined()packages/web-platform/web-rsbuild-server-middleware/README.md (1)
3-3: Fix grammar and articleUse “An Rsbuild … that provides a … on the dev server.” for clarity.
Apply:
-A rsbuild dev server middleware for providing mocked Lynx Web Platform installation on dev server. +An Rsbuild dev server middleware that provides a mocked Lynx Web Platform installation on the dev server.packages/web-platform/web-rsbuild-server-middleware/src/web/index.css (1)
5-8: Ensure custom element fills viewportCustom elements are inline by default; width/height won’t apply reliably. Make it block-level.
Apply:
lynx-view { + display: block; width: 100vw; height: 100vh; }packages/web-platform/web-tests/rspack.config.js (1)
20-21: Avoid ambiguous “main” naming; use descriptive middleware identifierRename to prevent confusion with the “main” entry chunk and improve readability.
Apply:
-const main = createWebVirtualFilesMiddleware('/middleware'); +const webMiddleware = createWebVirtualFilesMiddleware('/middleware'); ... - middlewares.push(main); + middlewares.push(webMiddleware);Optional: mount with explicit devServer metadata for clarity:
middlewares.push({ name: 'web-virtual-files', path: '/middleware', middleware: webMiddleware, });Also applies to: 208-209
packages/web-platform/web-rsbuild-server-middleware/Notice.txt (1)
1-1: Standardize NOTICE filename across repoRepo contains a top-level
NOTICEand many packageNotice.txtfiles (including packages/web-platform/web-rsbuild-server-middleware/Notice.txt). Choose one convention (prefer uppercaseNOTICEfor Apache attribution), rename files and update any references.packages/web-platform/web-tests/tests/middleware.spec.ts (2)
38-47: Use an absolute path for stability ingoto.Prefix the path with
/to avoid baseURL/path merging issues in different runners.- let url = `middleware?casename=/dist/${testname}/index.web.json`; + let url = `/middleware?casename=/dist/${testname}/index.web.json`;
55-64: Prefer auto‑waiting assertions over fixed sleeps.Replace manual
wait(100)with Playwright’s auto-waiting assertions to reduce flakiness.- await wait(100); - const target = page.locator('#target'); - await target.click(); - await wait(100); - await expect(await target.getAttribute('style')).toContain('green'); - await target.click(); - await wait(100); - await expect(await target.getAttribute('style')).toContain('pink'); + const target = page.locator('#target'); + await target.click(); + await expect(target).toHaveAttribute('style', /green/); + await target.click(); + await expect(target).toHaveAttribute('style', /pink/);packages/web-platform/web-rsbuild-server-middleware/vitest.config.ts (1)
5-7: Capture nested tests too.Broaden the include pattern to match nested spec files under test/.
- include: ['**/test/*.spec.ts'], + include: ['**/test/**/*.spec.ts'],packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
8-16: Consider rendering a QR for the web URL as well (consistency).Currently only the Lynx URL gets a QR. Printing a QR for web can help mobile-to-desktop handoff and keeps output consistent.
export default function showQRCode(lynxUrl?: string, webUrl?: string): void { if (lynxUrl) { log.info(color.green('Scan with Lynx')) log.success(renderUnicodeCompact(lynxUrl)) log.success('Lynx Explorer: ' + lynxUrl) } if (webUrl) { - log.success('Lynx Web Platform: ' + webUrl) + log.info(color.green('Open in Browser')) + log.success(renderUnicodeCompact(webUrl)) + log.success('Lynx Web Platform: ' + webUrl) } }packages/web-platform/web-rsbuild-server-middleware/test/basic.spec.ts (1)
19-26: Avoid double slashes in request URLs.Sanitize the path or let the helper handle leading slashes uniformly.
-const GET = async (path: string) => { +const GET = async (path: string) => { + path = path.startsWith('/') ? path.slice(1) : path; // Make an HTTP request over the IPC socket const result = await fetch(`http://localhost:${port}/${path}`); if (result.status !== 200) { throw new Error(`Request failed with status ${result.status}`); } return result; };packages/web-platform/web-rsbuild-server-middleware/package.json (1)
14-16: Prefertypesovertypingsand consider exports map.For consistency with the repo and TypeScript conventions, use
types. Optionally add anexportsmap to control entry points.- "main": "dist/index.js", - "typings": "dist/node/index.d.ts", + "main": "dist/index.js", + "types": "dist/node/index.d.ts",Optionally:
{ "exports": { ".": { "types": "./dist/node/index.d.ts", "default": "./dist/index.js" } } }packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
63-76: Correct Content-Length calculation and disable caching in devFor Buffers, use .length; also set no-store to avoid stale assets during dev.
Apply this diff:
- res.setHeader('Content-Length', Buffer.byteLength(fileContent)); + const len = typeof fileContent === 'string' + ? Buffer.byteLength(fileContent) + : fileContent.length; + res.setHeader('Content-Length', len); res.setHeader('Content-Type', contextType); + res.setHeader('Cache-Control', 'no-store');packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
71-74: Simplify types for value/devUrlsvalue.lynx is never a cancel token; it’s set to URLs only. Remove symbol from types to avoid unnecessary casts and confusion.
Apply this diff:
- value: { lynx: string | symbol | undefined, web: string | undefined }, - devUrls: { lynx: Record<string, string | symbol>, web: string }, + value: { lynx: string | undefined, web: string | undefined }, + devUrls: { lynx: Record<string, string>, web: string },
85-89: Fix loop condition; current use of isCancel on URLs is a no-opisCancel(value.lynx/web) always returns false for strings. Use an unconditional loop with internal breaks.
Apply this diff:
- while ( - !(value.lynx && isCancel(value.lynx) || value.web && isCancel(value.web)) - ) { + while (true) {The existing breaks on isCancel(name)/'q'/restart already control loop exit.
157-159: Drop unnecessary casts after type simplificationOnce value types are simplified to strings, these casts are not needed.
- await options.onPrint?.(value.lynx as string | undefined, value.web) - showQRCode(value.lynx as string, value.web) + await options.onPrint?.(value.lynx, value.web) + showQRCode(value.lynx, value.web)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.changeset/config.json(1 hunks).changeset/crazy-jeans-hunt.md(1 hunks).changeset/slow-radios-rescue.md(1 hunks).changeset/tricky-lamps-battle.md(1 hunks)examples/react/lynx.config.js(2 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(2 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(1 hunks)packages/rspeedy/plugin-qrcode/package.json(1 hunks)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/index.ts(4 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(8 hunks)packages/rspeedy/plugin-qrcode/src/showQRCode.ts(1 hunks)packages/rspeedy/plugin-qrcode/test/preview.test.ts(5 hunks)packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts(2 hunks)packages/web-platform/tsconfig.json(1 hunks)packages/web-platform/web-rsbuild-server-middleware/LICENSE.txt(1 hunks)packages/web-platform/web-rsbuild-server-middleware/Notice.txt(1 hunks)packages/web-platform/web-rsbuild-server-middleware/README.md(1 hunks)packages/web-platform/web-rsbuild-server-middleware/package.json(1 hunks)packages/web-platform/web-rsbuild-server-middleware/rsbuild.config.ts(1 hunks)packages/web-platform/web-rsbuild-server-middleware/rslib.config.ts(1 hunks)packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts(1 hunks)packages/web-platform/web-rsbuild-server-middleware/src/web/index.css(1 hunks)packages/web-platform/web-rsbuild-server-middleware/src/web/index.ts(1 hunks)packages/web-platform/web-rsbuild-server-middleware/test/basic.spec.ts(1 hunks)packages/web-platform/web-rsbuild-server-middleware/tsconfig.json(1 hunks)packages/web-platform/web-rsbuild-server-middleware/turbo.json(1 hunks)packages/web-platform/web-rsbuild-server-middleware/vitest.config.ts(1 hunks)packages/web-platform/web-tests/package.json(1 hunks)packages/web-platform/web-tests/rspack.config.js(2 hunks)packages/web-platform/web-tests/tests/middleware.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/tricky-lamps-battle.md.changeset/crazy-jeans-hunt.md.changeset/slow-radios-rescue.md
packages/web-platform/web-tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Playwright E2E tests in the web platform’s web-tests suite
Files:
packages/web-platform/web-tests/rspack.config.jspackages/web-platform/web-tests/tests/middleware.spec.tspackages/web-platform/web-tests/package.json
🧠 Learnings (14)
📚 Learning: 2025-09-22T11:07:32.317Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1807
File: packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts:44-47
Timestamp: 2025-09-22T11:07:32.317Z
Learning: Development server middleware packages (like rsbuild dev server middleware) can appropriately use permissive CORS settings like "Access-Control-Allow-Origin: *" since they are intended for local development and testing, not production use.
Applied to files:
packages/web-platform/web-rsbuild-server-middleware/README.md
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/web-platform/web-rsbuild-server-middleware/README.mdpackages/rspeedy/core/src/plugins/dev.plugin.ts.changeset/crazy-jeans-hunt.mdpackages/web-platform/web-rsbuild-server-middleware/package.jsonpackages/rspeedy/plugin-qrcode/package.json
📚 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, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
examples/react/lynx.config.jspackages/rspeedy/core/src/plugins/dev.plugin.ts.changeset/crazy-jeans-hunt.md.changeset/slow-radios-rescue.md.changeset/config.jsonpackages/web-platform/web-rsbuild-server-middleware/package.jsonpackages/web-platform/web-tests/package.json
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
examples/react/lynx.config.jspackages/web-platform/web-rsbuild-server-middleware/vitest.config.ts
📚 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/tricky-lamps-battle.md.changeset/crazy-jeans-hunt.md.changeset/slow-radios-rescue.md.changeset/config.json
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/tricky-lamps-battle.md.changeset/config.json
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/tricky-lamps-battle.md.changeset/crazy-jeans-hunt.md.changeset/slow-radios-rescue.md.changeset/config.json
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts.changeset/config.json
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts.changeset/crazy-jeans-hunt.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/slow-radios-rescue.md
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
.changeset/config.jsonpackages/web-platform/web-rsbuild-server-middleware/package.json
📚 Learning: 2025-09-09T12:38:10.450Z
Learnt from: CR
PR: lynx-family/lynx-stack#0
File: AGENTS.md:0-0
Timestamp: 2025-09-09T12:38:10.450Z
Learning: Applies to packages/web-platform/web-tests/** : Place Playwright E2E tests in the web platform’s web-tests suite
Applied to files:
packages/web-platform/web-rsbuild-server-middleware/test/basic.spec.tspackages/web-platform/web-tests/tests/middleware.spec.tspackages/web-platform/web-tests/package.jsonpackages/web-platform/web-rsbuild-server-middleware/vitest.config.ts
📚 Learning: 2025-08-14T12:54:51.143Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: .changeset/brave-melons-add.md:1-7
Timestamp: 2025-08-14T12:54:51.143Z
Learning: In the lynx-family/lynx-stack repository, packages use 0.x.x versioning where minor version bumps indicate breaking changes (not major bumps), following pre-1.0 semantic versioning conventions.
Applied to files:
packages/web-platform/web-rsbuild-server-middleware/Notice.txt
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-rsbuild-server-middleware/vitest.config.ts
🧬 Code graph analysis (10)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
packages/rspeedy/core/src/utils/is-lynx.ts (1)
isLynx(7-11)
packages/web-platform/web-rsbuild-server-middleware/src/web/index.ts (1)
packages/web-platform/web-tests/server.js (1)
lynxView(16-33)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (2)
packages/web-platform/web-tests/rspack.config.js (2)
__filename(14-14)__dirname(15-15)packages/web-platform/web-style-transformer/scripts/build.js (1)
packageRoot(7-7)
packages/web-platform/web-tests/rspack.config.js (1)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)
packages/rspeedy/plugin-qrcode/test/preview.test.ts (1)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
pluginQRCode(93-180)
packages/web-platform/web-rsbuild-server-middleware/rsbuild.config.ts (1)
packages/web-platform/web-tests/rspack.config.js (1)
__dirname(15-15)
packages/web-platform/web-rsbuild-server-middleware/test/basic.spec.ts (1)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
generateDevUrls(10-69)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-17)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
registerConsoleShortcuts(30-67)
🪛 GitHub Check: code-style-check
packages/rspeedy/plugin-qrcode/src/index.ts
[failure] 13-13:
Cannot find module '@lynx-js/web-rsbuild-server-middleware' or its corresponding type declarations.
⏰ 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). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (32)
.changeset/crazy-jeans-hunt.md (1)
1-6: Good patch entry.Clear fix note for web platform error.
.changeset/tricky-lamps-battle.md (1)
1-15: Feature changeset reads well.Message and example align with adding web preview support.
examples/react/lynx.config.js (3)
8-13: Two entries pointing to the same file—intentional?Duplicating entry to the same path can create duplicate bundles unless used to exercise multi-entry behavior.
If unintentional, remove main2:
entry: { main: './src/index.tsx', - main2: './src/index.tsx', },
16-21: Confirm schema callback signature with new QR code URLs.If the plugin now passes a composite URLs object, adjust the example accordingly; otherwise current string mapping is fine.
If composite is passed:
- schema(url) { - return `${url}?fullscreen=true`; - }, + schema(urls) { + if (typeof urls === 'string') return `${urls}?fullscreen=true`; + return { + ...urls, + web: `${urls.web}?fullscreen=true`, + lynx: Object.fromEntries( + Object.entries(urls.lynx).map(([k, v]) => [k, `${v}?fullscreen=true`]), + ), + }; + },
26-29: Environments block LGTM.Matches the new web and lynx split.
packages/rspeedy/core/src/plugins/dev.plugin.ts (2)
15-15: Import of isLynx is appropriate.Sets up environment gating cleanly.
123-171: HMR aliases and ProvidePlugin gated to Lynx—LGTM.Matches new behavior and corresponding tests; chaining looks correct.
Please confirm web still gets a working dev experience without these aliases (no unintended regressions in live reload/HMR expectations for web).
packages/web-platform/web-rsbuild-server-middleware/rslib.config.ts (2)
22-32: Type-check/publish plugins usage looks good.Conditional ATTW and publint integration is appropriate.
12-14: dts.tsgo is a valid (experimental) Rslib option — no change required
Rslib supports dts.tsgo (boolean) to enable tsgo/@typescript/native-preview for faster .d.ts generation; it defaults to false and requires installing @typescript/native-preview and a recent Node.js runtime..changeset/slow-radios-rescue.md (1)
1-6: Approve changeset — package is public; patch OK.
package.json shows "private": false, so a patch changeset for an initial publish is appropriate.packages/web-platform/tsconfig.json (1)
25-26: LGTM — project reference wiredReference to web-rsbuild-server-middleware is correctly added.
packages/web-platform/web-rsbuild-server-middleware/LICENSE.txt (1)
1-203: LGTM — Apache-2.0 includedLicense content present and consistent with repo licensing.
packages/web-platform/web-tests/rspack.config.js (1)
13-14: LGTM — imports middlewareImport is correct and scoped to devServer usage.
packages/web-platform/web-tests/package.json (1)
40-41: LGTM — Playwright tests target the mounted "/middleware" pathrspack.config.js mounts createWebVirtualFilesMiddleware('/middleware'); middleware.spec.ts navigates to "middleware?casename=…"(relative), so the tests will hit the same subPath (see packages/web-platform/web-tests/rspack.config.js and packages/web-platform/web-tests/tests/middleware.spec.ts).
packages/web-platform/web-tests/tests/middleware.spec.ts (1)
49-53: Tests are correctly placed under web-tests suite.Placement aligns with repository testing conventions.
packages/rspeedy/plugin-qrcode/package.json (1)
50-52: Dependency addition looks correct (runtime usage).Using a runtime dependency makes sense for dev-server integration.
Please confirm this package is intended as a runtime dep (not dev/peer). If it’s only needed during development of this plugin, consider devDependency. Also ensure a changeset is added for this package if publishable versions change due to the new dep.
.changeset/config.json (1)
18-20: Adding the new public package to the fixed group looks good.Ensure this PR includes a changeset for introducing @lynx-js/web-rsbuild-server-middleware and any packages with meaningful user-facing changes.
packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
8-16: Call sites updated to the new signature (lynxUrl?, webUrl?)showQRCode is called as showQRCode(lynxUrl, webUrl) in packages/rspeedy/plugin-qrcode/src/shortcuts.ts (lines ~56 and ~158).
packages/web-platform/web-rsbuild-server-middleware/rsbuild.config.ts (1)
20-22: Confirm config option name and intent.
- output.overrideBrowserslist is a valid rsbuild option and will override .browserslistrc / package.json; rsbuild recommends using a .browserslistrc for portability.
- Verify assetPrefix 'http://lynx-web-core-mocked.localhost/' exactly matches the replacement string used by the middleware.
Location: packages/web-platform/web-rsbuild-server-middleware/rsbuild.config.ts (lines 20–22)
packages/rspeedy/plugin-qrcode/src/index.ts (5)
106-110: Middleware mount is correct; confirm no route conflictsMounting at '/web' looks fine. Verify no existing dev-server routes use '/web' and ensure this executes before any catch-all/static handlers that could shadow it.
111-113: LGTMProd preview path now prints QR consistently using the environments map.
132-133: LGTMDev compile hook guards against duplicate prints; good integration with environments map and server port.
147-159: Entry aggregation looks goodUnion of lynx/web entries with unique deduping is correct.
160-177: Console shortcuts registration looks goodStructured entries { all, lynx, web } flow matches updated consumers.
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
38-47: Base URL normalization is solidReplacing and falling back to localhost is robust.
64-69: LGTMReturn shape { lynx, web } and lynx schema handling are correct.
packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts (1)
41-57: Tests updated correctly for new APIentries shape and onPrint(lynx, web) expectations align with implementation.
packages/rspeedy/plugin-qrcode/test/preview.test.ts (2)
180-214: Preview without entry behavior verifiedNo QR render when entries empty; updated port range is fine.
215-248: Preview without lynx env behavior verifiedSkips QR rendering as expected when lynx/web not configured.
packages/web-platform/web-rsbuild-server-middleware/src/web/index.ts (2)
11-15: LGTMInit of lynx-view and reading casename from query looks correct.
16-44: LGTMLocalStorage-backed initData/globalProps with safe parsing is sound.
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
39-41: Default entry selection is fine; guard is upstreamRelies on index.ts ensuring there’s at least one lynx/web entry.
| const outputPathname = new URL( | ||
| name.replace('[name]', entry).replace('[platform]', 'web'), | ||
| base, | ||
| ).pathname | ||
| const web = new URL( | ||
| `/web?casename=${outputPathname}`, | ||
| base, | ||
| ).toString() | ||
|
|
There was a problem hiding this comment.
Encode query parameter to avoid broken URLs
outputPathname can contain slashes and special chars; it should be URL-encoded when used as a query param.
Apply this diff:
- const web = new URL(
- `/web?casename=${outputPathname}`,
- base,
- ).toString()
+ const web = new URL(
+ `/web?casename=${encodeURIComponent(outputPathname)}`,
+ base,
+ ).toString()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const outputPathname = new URL( | |
| name.replace('[name]', entry).replace('[platform]', 'web'), | |
| base, | |
| ).pathname | |
| const web = new URL( | |
| `/web?casename=${outputPathname}`, | |
| base, | |
| ).toString() | |
| const outputPathname = new URL( | |
| name.replace('[name]', entry).replace('[platform]', 'web'), | |
| base, | |
| ).pathname | |
| const web = new URL( | |
| `/web?casename=${encodeURIComponent(outputPathname)}`, | |
| base, | |
| ).toString() |
🤖 Prompt for AI Agents
In packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts around lines 55 to 63,
the code inserts outputPathname directly into the query string which can break
URLs when outputPathname contains slashes or special characters; update the code
to URL-encode the path before adding it to the query (e.g. use
encodeURIComponent(outputPathname)) so the generated /web?casename=... parameter
is safe and correctly formed.
| import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Fix missing dependency/exports for web middleware import
Type-check fails: Cannot find module '@lynx-js/web-rsbuild-server-middleware'. Ensure the package is part of the workspace, exported correctly, and added as a dependency of this plugin. If the package exposes a node entry, import it explicitly.
Apply one of:
- import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware'
+ import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware/node'Or add dependency and proper exports/types to the workspace package.
Run this script to verify dependency wiring and exports:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check plugin dependency..."
PLUGIN_PKG=$(fd -a -t f '^package\.json$' | rg -n 'packages/rspeedy/plugin-qrcode/package.json' -N | cut -d: -f1 || true)
WEB_PKG=$(fd -a -t f '^package\.json$' | rg -n 'packages/web-platform/web-rsbuild-server-middleware/package.json' -N | cut -d: -f1 || true)
if [[ -z "$PLUGIN_PKG" || -z "$WEB_PKG" ]]; then
echo "Missing package.json files. Found:"
echo "plugin: $PLUGIN_PKG"
echo "web : $WEB_PKG"
exit 1
fi
echo "Plugin depends on web middleware?"
jq -r '.dependencies["@lynx-js/web-rsbuild-server-middleware"], .devDependencies["@lynx-js/web-rsbuild-server-middleware"]' "$PLUGIN_PKG"
echo "Web middleware package name and exports:"
jq -r '.name, .exports, .main, .types' "$WEB_PKG"
echo "Look for a 'node' export subpath:"
jq -r '.exports["./node"], .exports["node"]' "$WEB_PKG" 2>/dev/null || trueLength of output: 225
Expose a node entry and type declarations from @lynx-js/web-rsbuild-server-middleware
- packages/rspeedy/plugin-qrcode/package.json depends on @lynx-js/web-rsbuild-server-middleware (workspace:*).
- packages/web-platform/web-rsbuild-server-middleware/package.json has no "exports" and no "types" (main: dist/index.js) and does not expose a "./node" subpath — add an "exports" mapping (include "./node" if you intend to import '/node') and publish TypeScript declarations or set "types" so the module can be resolved by type-checking.
🧰 Tools
🪛 GitHub Check: code-style-check
[failure] 13-13:
Cannot find module '@lynx-js/web-rsbuild-server-middleware' or its corresponding type declarations.
🤖 Prompt for AI Agents
In packages/rspeedy/plugin-qrcode/src/index.ts around lines 13 to 14, the import
from '@lynx-js/web-rsbuild-server-middleware' fails type-resolution because the
source package (packages/web-platform/web-rsbuild-server-middleware) does not
declare "exports" (including a "./node" subpath if you intend to import a
node-specific entry) nor does it publish TypeScript declarations or set "types".
Fix package.json of web-rsbuild-server-middleware by adding an "exports" map
that exposes the default entry and a "./node" subpath (pointing to the built JS
files), and add a "types" field or publish .d.ts files from the dist so
TypeScript and consumers can resolve imports and typings correctly; then
rebuild/publish the package and update workspace deps if needed.
6b0e0a2 to
709e3c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.changeset/tricky-lamps-battle.md (2)
2-3: Use a minor bump for a user-facing featureThis enables a new user-facing capability (web platform preview). Recommend
minorinstead ofpatch.--- -"@lynx-js/qrcode-rsbuild-plugin": patch +"@lynx-js/qrcode-rsbuild-plugin": minor ---
2-3: Add changeset entries for other touched packages (core dev plugin)Core dev plugin changes (gating HMR by environment) affect users and live under src/**. Add a changeset (patch) for the core package to satisfy CI and release the behavior change.
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
102-121: Tighten the “no HMR for web” assertionsUsing
not.toHaveProperty(key, value)passes if the value differs, even when the property exists. If the intent is “property absent”, omit the second arg.- expect(config.resolve?.alias).not.toHaveProperty( - '@rspack/core/hot/dev-server', - expect.stringContaining('hot/dev-server.js'.replaceAll('/', path.sep)), - ) - expect(config.resolve?.alias).not.toHaveProperty( - '@lynx-js/webpack-dev-transport/client', - expect.stringContaining( - 'packages/webpack/webpack-dev-transport'.replaceAll('/', path.sep), - ), - ) + expect(config.resolve?.alias).not.toHaveProperty('@rspack/core/hot/dev-server') + expect(config.resolve?.alias).not.toHaveProperty('@lynx-js/webpack-dev-transport/client')packages/rspeedy/plugin-qrcode/src/shortcuts.ts (2)
111-125: Show environment-appropriate hints in “Switch entries”Don’t hint a Lynx URL for web-only entries.
- options: options.entries.all.map(entry => { - const newDevUrls = generateDevUrls( + options: options.entries.all.map(entry => { + const newDevUrls = generateDevUrls( options.api, entry, options.schema, options.port, ) - return { - value: entry, - label: entry, - hint: (newDevUrls.lynx[currentSchema] ?? newDevUrls.web), - } + const lynxEnabled = options.entries.lynx.includes(entry) + const webEnabled = options.entries.web.includes(entry) + return { + value: entry, + label: entry, + hint: lynxEnabled + ? newDevUrls.lynx[currentSchema] + : webEnabled + ? newDevUrls.web + : undefined, + } }),
85-89: Loop guard uses cancel checks on URL values
value.lynx/webare URLs, not clack cancel symbols. The inner menu break handles cancel/quit; this outer guard can be simplified.- while ( - !(value.lynx && isCancel(value.lynx) || value.web && isCancel(value.web)) - ) { + while (true) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.changeset/crazy-jeans-hunt.md(1 hunks).changeset/tricky-lamps-battle.md(1 hunks)examples/react/lynx.config.js(2 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(2 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(1 hunks)packages/rspeedy/plugin-qrcode/package.json(1 hunks)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/index.ts(4 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(8 hunks)packages/rspeedy/plugin-qrcode/src/showQRCode.ts(1 hunks)packages/rspeedy/plugin-qrcode/test/preview.test.ts(5 hunks)packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts
- .changeset/crazy-jeans-hunt.md
- packages/rspeedy/plugin-qrcode/package.json
- examples/react/lynx.config.js
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/tricky-lamps-battle.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
📚 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/tricky-lamps-battle.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/tricky-lamps-battle.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/tricky-lamps-battle.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.tspackages/rspeedy/plugin-qrcode/src/index.ts
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
📚 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, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/rspeedy/core/src/plugins/dev.plugin.ts
🧬 Code graph analysis (5)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
packages/rspeedy/core/src/utils/is-lynx.ts (1)
isLynx(7-11)
packages/rspeedy/plugin-qrcode/test/preview.test.ts (1)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
pluginQRCode(93-180)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
generateDevUrls(10-69)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-17)
packages/rspeedy/core/test/plugins/dev.plugin.test.ts (1)
packages/rspeedy/core/test/createStubRspeedy.ts (1)
createStubRspeedy(20-64)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
registerConsoleShortcuts(30-67)
🪛 GitHub Check: code-style-check
packages/rspeedy/plugin-qrcode/src/index.ts
[failure] 13-13:
Cannot find module '@lynx-js/web-rsbuild-server-middleware' or its corresponding type declarations.
⏰ 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). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (11)
packages/rspeedy/core/src/plugins/dev.plugin.ts (1)
123-171: LGTM: Correctly gate HMR wiring to Lynx env onlyAliasing and ProvidePlugin are now applied only when
isLynx(environment). This prevents accidental HMR client injection for web. Looks good.Please ensure default tests still run under a Lynx environment by default (they appear to).
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
38-47: Nice fallback for malformed assetPrefixGraceful fallback to
http://localhost:<port>/keeps preview resilient ifdev.assetPrefixis invalid.
55-63: Encode query parameter to avoid broken URLs
outputPathnamecan contain slashes/special chars; it should be URL-encoded when used in a query param.- const web = new URL( - `/web?casename=${outputPathname}`, - base, - ).toString() + const web = new URL( + `/web?casename=${encodeURIComponent(outputPathname)}`, + base, + ).toString()packages/rspeedy/plugin-qrcode/src/index.ts (3)
106-110: LGTM: Web middleware registration is properly scopedMiddleware is mounted only when a web environment exists and before dev server starts. Good boundary.
147-171: LGTM: Multi-environment entry handlingCollecting
lynxandwebentries and early-exiting when none exist aligns with the new dual-URL flow.
13-14: Expose./nodeentry and types in web-rsbuild-server-middleware
- In packages/web-rsbuild-server-middleware/package.json add an
exports["./node"]entry pointing to the Node output and publish its.d.ts.- Then update the import in packages/rspeedy/plugin-qrcode/src/index.ts:
-import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware' +import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware/node'packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
8-16: LGTM: Dual output without changing QR semanticsKeeps QR rendering for Lynx only and logs the Web URL separately, matching test expectations.
packages/rspeedy/plugin-qrcode/test/preview.test.ts (2)
180-213: LGTM: “no entry” preview path coveredTest ensures no QR is rendered when entries are empty. Matches plugin skip logic.
215-248: LGTM: No Lynx environment should not render QR or exitConfirms no QR printed and process not exited when
environments.lynxis absent.packages/rspeedy/plugin-qrcode/src/shortcuts.ts (2)
170-181: Gate URLs by environment presence when switching entry/schemaAvoid showing a Lynx URL for a web-only entry (and vice versa).
- function getCurrentUrl(): { - lynx: string | undefined - web: string | undefined - } { - const { lynx, web } = generateDevUrls( + function getCurrentUrl(): { lynx: string | undefined; web: string | undefined } { + const { lynx, web } = generateDevUrls( options.api, currentEntry, options.schema, options.port, ) - return { lynx: lynx[currentSchema], web } + const lynxEnabled = options.entries.lynx.includes(currentEntry) + const webEnabled = options.entries.web.includes(currentEntry) + return { + lynx: lynxEnabled ? lynx[currentSchema] : undefined, + web: webEnabled ? web : undefined, + } }
48-56: Initial print correctly gated per environmentNice touch to avoid emitting Lynx QR for web-only entries and vice versa.
PupilTong
force-pushed
the
p/hw/web-qrcode-url
branch
from
709e3c0 to
ede6c13
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
.changeset/tricky-lamps-battle.md (1)
2-3: Use a minor bump for a feature additionThis is a new feature; recommend "minor" instead of "patch" per semver.
---- -"@lynx-js/qrcode-rsbuild-plugin": patch +"@lynx-js/qrcode-rsbuild-plugin": minor ----packages/web-platform/web-rsbuild-server-middleware/package.json (1)
15-15: Also set the "types" field for TS tooling compatibilityTypeScript recognizes both, but "types" is the common field; keep "typings" if you prefer, but add "types" for consistency across the repo.
"main": "dist/index.js", - "typings": "dist/index.d.ts", + "typings": "dist/index.d.ts", + "types": "dist/index.d.ts",packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
8-16: Optionally render a QR for the web URLIf scanning to open the web preview on a phone is useful, also render a QR when webUrl is provided.
export default function showQRCode(lynxUrl?: string, webUrl?: string): void { if (lynxUrl) { log.info(color.green('Scan with Lynx')) log.success(renderUnicodeCompact(lynxUrl)) log.success('Lynx Explorer: ' + lynxUrl) } if (webUrl) { - log.success('Lynx Web Platform: ' + webUrl) + log.info(color.cyan('Open in browser')) + log.success(renderUnicodeCompact(webUrl)) + log.success('Lynx Web Platform: ' + webUrl) } }packages/rspeedy/plugin-qrcode/src/index.ts (1)
112-112: Also serve middleware in preview mode (if needed)If the web preview requires the same assets in rsbuild preview, add the middleware for prod server too.
- api.onAfterStartProdServer(async ({ environments, port }) => { - await main(environments, port) - }) + api.onAfterStartProdServer(async ({ environments, server, port }) => { + if (environments['web']) { + server.middlewares.use(createWebVirtualFilesMiddleware('/web')) + } + await main(environments, port) + })packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
39-41: Avoid non-null assertion; handle empty entries gracefullyGuard when both entries.lynx and entries.web are empty to prevent runtime crash and provide a no-op unregistration.
- // keep the default entry to be lynx explorer entry if exists - const currentEntry = (options.entries.lynx[0] ?? options.entries.web[0])! + // keep the default entry to be lynx explorer entry if exists + const currentEntry = options.entries.lynx[0] ?? options.entries.web[0] + if (!currentEntry) { + await options.onPrint?.() + showQRCode() + return () => {} + }
111-126: Gate entry hints by environment membershipWhen listing entries, avoid showing a Lynx URL for web-only entries (and vice versa). Compute hint with per-entry gating.
- options: options.entries.all.map(entry => { - const newDevUrls = generateDevUrls( + options: options.entries.all.map(entry => { + const newDevUrls = generateDevUrls( options.api, entry, options.schema, options.port, - ) - return { - value: entry, - label: entry, - hint: (newDevUrls.lynx[currentSchema] ?? newDevUrls.web), - } + ) + const hasLynx = options.entries.lynx.includes(entry) + const hasWeb = options.entries.web.includes(entry) + const hint = hasLynx + ? newDevUrls.lynx[currentSchema] + : hasWeb + ? newDevUrls.web + : undefined + return { value: entry, label: entry, hint } }),
140-146: Avoid shadowing outer devUrls; rename for clarityThe local variable shadows the parameter, increasing cognitive load.
- const devUrls = generateDevUrls( + const newDevUrls = generateDevUrls( options.api, currentEntry, options.schema, options.port, ) - const selection = await selectFn(Object.keys(devUrls.lynx).length)({ + const selection = await selectFn(Object.keys(newDevUrls.lynx).length)({ message: 'Select schema', - options: Object.entries(devUrls.lynx).map(([name, url]) => ({ + options: Object.entries(newDevUrls.lynx).map(([name, url]) => ({ value: name, label: name, hint: url, })),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.changeset/crazy-jeans-hunt.md(1 hunks).changeset/tricky-lamps-battle.md(1 hunks)examples/react/lynx.config.js(1 hunks)packages/rspeedy/core/src/plugins/dev.plugin.ts(3 hunks)packages/rspeedy/core/test/plugins/dev.plugin.test.ts(1 hunks)packages/rspeedy/plugin-qrcode/package.json(1 hunks)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/index.ts(4 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(8 hunks)packages/rspeedy/plugin-qrcode/src/showQRCode.ts(1 hunks)packages/rspeedy/plugin-qrcode/test/preview.test.ts(5 hunks)packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts(2 hunks)packages/rspeedy/plugin-qrcode/tsconfig.build.json(1 hunks)packages/rspeedy/plugin-qrcode/tsconfig.json(1 hunks)packages/web-platform/web-rsbuild-server-middleware/package.json(2 hunks)packages/web-platform/web-rsbuild-server-middleware/rslib.config.ts(0 hunks)packages/web-platform/web-rsbuild-server-middleware/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web-platform/web-rsbuild-server-middleware/rslib.config.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/rspeedy/plugin-qrcode/test/preview.test.ts
- packages/rspeedy/core/src/plugins/dev.plugin.ts
- packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts
- .changeset/crazy-jeans-hunt.md
- examples/react/lynx.config.js
- packages/rspeedy/core/test/plugins/dev.plugin.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/tricky-lamps-battle.md
🧠 Learnings (3)
📓 Common learnings
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, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
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.
📚 Learning: 2025-08-27T08:10:09.932Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.932Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.
Applied to files:
packages/rspeedy/plugin-qrcode/package.json
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/plugin-qrcode/src/index.ts
🧬 Code graph analysis (2)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
generateDevUrls(10-69)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-17)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
registerConsoleShortcuts(30-67)
⏰ 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). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (12)
packages/rspeedy/plugin-qrcode/package.json (1)
50-52: Dependency wiring for web middleware looks correctRuntime dep added; aligns with new middleware usage in the plugin.
If you intend to import from the package root, confirm that @lynx-js/web-rsbuild-server-middleware’s root index exports createWebVirtualFilesMiddleware (see comment on index.ts for a verification script).
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
55-63: Encode query parameter to avoid broken URLs- const web = new URL( - `/web?casename=${outputPathname}`, - base, - ).toString() + const web = new URL( + `/web?casename=${encodeURIComponent(outputPathname)}`, + base, + ).toString()packages/rspeedy/plugin-qrcode/src/index.ts (4)
106-110: Good: middleware gated to web environmentRegistering the virtual files middleware only when environments.web exists is sound.
132-132: LGTM: single print on first successful buildAvoids duplicate QR output on rebuilds.
148-177: Entry computation and shortcut registration look correctMerges lynx/web entries, handles empty case, and wires cleanup on dev-server close.
If rsbuild exposes a separate close hook for preview/prod servers, consider mirroring cleanup there to avoid leaks when running preview.
13-14: Confirm root export of createWebVirtualFilesMiddlewareThis import assumes the package root re-exports the Node entry. If not, either add an exports map in the middleware package or import from a subpath you expose (e.g. './node').
Verification script to check the middleware package source exports:
If missing, consider adding an exports map:
+ "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + }, + "./node": { + "types": "./dist/node/index.d.ts", + "default": "./dist/node/index.js" + } + },packages/rspeedy/plugin-qrcode/tsconfig.build.json (1)
10-10: Ensure referenced project is compositeTypeScript project references require the target tsconfig to have "composite": true; please verify the middleware tsconfig is set accordingly.
packages/web-platform/web-rsbuild-server-middleware/package.json (1)
26-26: Verify build emits .d.ts to dist/With rslib.config.ts removed and a consolidated "rsbuild build", ensure declaration files still land in dist to satisfy "typings"/"types".
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (2)
48-56: LGTM: initial per-environment gating before printCorrectly gates Lynx/Web URLs against current entry before printing/logging.
170-181: Gate URLs by environment when switching entry/schema (regression of prior fix)Reintroduces the issue of exposing a Lynx URL for web-only entries (and vice versa). Gate per entries.lynx/web.
function getCurrentUrl(): { lynx: string | undefined web: string | undefined } { const { lynx, web } = generateDevUrls( options.api, currentEntry, options.schema, options.port, ) - return { lynx: lynx[currentSchema], web } + const lynxEnabled = options.entries.lynx.includes(currentEntry) + const webEnabled = options.entries.web.includes(currentEntry) + return { + lynx: lynxEnabled ? lynx[currentSchema] : undefined, + web: webEnabled ? web : undefined, + } }packages/web-platform/web-rsbuild-server-middleware/tsconfig.json (1)
5-5: LGTM: composite build config now emits; node-only include is clearRemoving noEmit and scoping to src/node aligns with being a referenced project.
Please confirm declarations are emitted (either implicit via composite or explicit) for downstream type consumption. If not, add:
"compilerOptions": { "composite": true, "rootDir": "./src/node", "outDir": "./dist", "lib": ["ESNext"], + "declaration": true, + "declarationMap": true, },Also applies to: 9-9
packages/rspeedy/plugin-qrcode/tsconfig.json (1)
10-10: Validate project reference target and declaration emitReferencing the middleware’s tsconfig is fine; ensure it’s a composite project that emits declarations so TS project references work end-to-end.
- If middleware tsconfig isn’t emitting .d.ts, add "declaration": true (and optionally "declarationMap": true) there.
- Alternatively, point to its build tsconfig if that’s the convention across the repo.
Run to verify:
PupilTong
force-pushed
the
p/hw/web-qrcode-url
branch
from
ede6c13 to
bf282cb
Compare
CodSpeed Performance ReportMerging #1816 will degrade performances by 11.47%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
45 ms | 41.4 ms | +8.81% |
| ⚡ | basic-performance-div-100 |
6.2 ms | 5.9 ms | +5.09% |
| ❌ | basic-performance-small-css |
6.9 ms | 7.8 ms | -11.47% |
Footnotes
-
3 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. ↩
Web Explorer#5813 Bundle Size — 365.13KiB (0%).4ed1eae(current) vs 8983d4c main#5779(baseline) Bundle metrics
|
| Current #5813 |
Baseline #5779 |
|
|---|---|---|
 Initial JS |
145.05KiB |
145.05KiB |
| Initial CSS |
32KiB |
32KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
217(-0.91%) |
219 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.37%(+0.3%) |
3.36% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5813 |
Baseline #5779 |
|
|---|---|---|
| JS |
239.33KiB |
239.33KiB |
| Other |
93.8KiB |
93.8KiB |
| CSS |
32KiB |
32KiB |
Bundle analysis report Branch PupilTong:p/hw/web-qrcode-url Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
p/hw/web-qrcode-url
branch
from
bf282cb to
d791681
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
147-177: Sort entry keys and ensure cleanup on production exit
- Sort
lynxEntries,webEntries, andallEntriesfor deterministic ordering.- No
onCloseProdServerhook exists; invokeunregisterin theonAfterStartProdServerteardown (or attach it to the production server’s close event) to avoid leaking console shortcuts.- const lynxEntries = Object.keys(environments['lynx']?.entry ?? {}) - const webEntries = Object.keys(environments['web']?.entry ?? {}) - const allEntries = [ - ...new Set([ - ...lynxEntries, - ...webEntries, - ]), - ] + const lynxEntries = Object.keys(environments['lynx']?.entry ?? {}).sort() + const webEntries = Object.keys(environments['web']?.entry ?? {}).sort() + const allEntries = Array.from(new Set([...lynxEntries, ...webEntries])).sort()
🧹 Nitpick comments (6)
packages/rspeedy/plugin-qrcode/tsconfig.json (1)
10-10: Project reference consistency (build vs. base tsconfig)Consider referencing the middleware’s build tsconfig for consistency with the existing core reference, unless there’s a reason to target its base tsconfig.
If appropriate, apply:
- { "path": "../../web-platform/web-rsbuild-server-middleware/tsconfig.json" }, + { "path": "../../web-platform/web-rsbuild-server-middleware/tsconfig.build.json" },Also verify the referenced tsconfig has
"composite": true; otherwisetsc -bwill fail.packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
111-125: Entry selection hint should respect per‑environment availabilityShow the Lynx schema URL only for Lynx entries; otherwise show Web URL (if available).
Apply:
- return { - value: entry, - label: entry, - hint: (newDevUrls.lynx[currentSchema] ?? newDevUrls.web), - } + const lynxEnabled = options.entries.lynx.includes(entry) + const webEnabled = options.entries.web.includes(entry) + return { + value: entry, + label: entry, + hint: lynxEnabled + ? newDevUrls.lynx[currentSchema] + : webEnabled + ? newDevUrls.web + : undefined, + }.changeset/tricky-lamps-battle.md (1)
2-2: Semver bump likely should be minor, not patchThis adds a feature (web platform preview). Recommend bumping @lynx-js/qrcode-rsbuild-plugin to minor.
-"@lynx-js/qrcode-rsbuild-plugin": patch +"@lynx-js/qrcode-rsbuild-plugin": minorPlease confirm your repo’s semver policy for features.
packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts (1)
54-57: Strengthen assertion: check the first call explicitlyOptional: assert the first invocation’s args explicitly to avoid ambiguity when multiple calls occur.
Apply:
- expect(onPrint).toBeCalledWith( + expect(onPrint).toHaveBeenNthCalledWith(1, 'https://example.com/foo.lynx.bundle', undefined, )packages/rspeedy/plugin-qrcode/src/index.ts (2)
106-110: Confirm middleware mount path '/web' is correct and non-conflictingMounting at '/web' is reasonable; ensure this doesn’t clash with app routes or base/publicPath. Consider making the subpath configurable if needed.
112-113: Production: confirm that printing QR and console loop are desiredmain() is invoked for prod server too. Verify that interactive console shortcuts (prompts) are intended in prod; otherwise, guard to dev-only.
Apply:
- api.onAfterStartProdServer(async ({ environments, port }) => { - await main(environments, port) - }) + api.onAfterStartProdServer(async ({ environments, port }) => { + if (process.env.NODE_ENV !== 'production') return + await main(environments, port) + })Or, skip registering prompts entirely for prod.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/tricky-lamps-battle.md(1 hunks)examples/react/lynx.config.js(1 hunks)packages/rspeedy/plugin-qrcode/package.json(1 hunks)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/index.ts(4 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(8 hunks)packages/rspeedy/plugin-qrcode/src/showQRCode.ts(1 hunks)packages/rspeedy/plugin-qrcode/test/preview.test.ts(5 hunks)packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts(2 hunks)packages/rspeedy/plugin-qrcode/tsconfig.build.json(1 hunks)packages/rspeedy/plugin-qrcode/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rspeedy/plugin-qrcode/src/showQRCode.ts
- packages/rspeedy/plugin-qrcode/package.json
- packages/rspeedy/plugin-qrcode/test/preview.test.ts
- packages/rspeedy/plugin-qrcode/tsconfig.build.json
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/tricky-lamps-battle.md
🧠 Learnings (5)
📚 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, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
examples/react/lynx.config.js
📚 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/tricky-lamps-battle.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/tricky-lamps-battle.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/tricky-lamps-battle.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/rspeedy/plugin-qrcode/src/index.ts
🧬 Code graph analysis (2)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
generateDevUrls(10-69)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-17)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
registerConsoleShortcuts(30-67)
⏰ 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: test-rust / rustfmt
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: code-style-check
🔇 Additional comments (9)
examples/react/lynx.config.js (1)
20-23: LGTM: environments map addedThe environments map (web/lynx) aligns with the dual‑URL flow and web middleware wiring.
Please confirm the example runs with both Lynx and Web previews enabled in your local dev flow.
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
71-90: Remove dead cancel checks and unsafe casts; fix typesvalue.lynx/web are URLs, not cancel tokens; the loop condition is ineffective and types are overly permissive. Clean up types and loop condition; drop the casts.
Apply:
-async function loop( - options: Options, - value: { lynx: string | symbol | undefined, web: string | undefined }, - devUrls: { lynx: Record<string, string | symbol>, web: string }, - currentEntry: string, -) { +async function loop( + options: Options, + value: { lynx?: string; web?: string }, + devUrls: { lynx: Record<string, string>; web: string }, + currentEntry: string, +) { @@ - while ( - !(value.lynx && isCancel(value.lynx) || value.web && isCancel(value.web)) - ) { + while (true) { @@ - await options.onPrint?.(value.lynx as string | undefined, value.web) - showQRCode(value.lynx as string, value.web) + await options.onPrint?.(value.lynx, value.web) + showQRCode(value.lynx, value.web)Also applies to: 157-159
39-56: Initial defaulting and gating look goodDefaulting to a Lynx entry when present and gating initial lynx/web URLs by entry availability is correct.
170-181: Gate URLs by environment when switching entry/schemaAvoid returning a Lynx QR for a web‑only entry (and vice versa).
Apply:
- function getCurrentUrl(): { - lynx: string | undefined - web: string | undefined - } { + function getCurrentUrl(): { lynx?: string; web?: string } { const { lynx, web } = generateDevUrls( options.api, currentEntry, options.schema, options.port, ) - return { lynx: lynx[currentSchema], web } + const lynxEnabled = options.entries.lynx.includes(currentEntry) + const webEnabled = options.entries.web.includes(currentEntry) + return { + lynx: lynxEnabled ? lynx[currentSchema] : undefined, + web: webEnabled ? web : undefined, + } }.changeset/tricky-lamps-battle.md (1)
5-15: LGTM: changeset content and example configGood, concise release note with a minimal config example that mirrors the code changes.
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
55-63: Encode query parameter to prevent malformed URLsoutputPathname may contain slashes and special characters; encode it before embedding in the query string.
Apply:
- const web = new URL( - `/web?casename=${outputPathname}`, - base, - ).toString() + const web = new URL( + `/web?casename=${encodeURIComponent(outputPathname)}`, + base, + ).toString()
15-15: Return type change verified
AllgenerateDevUrlscall sites handle the{ lynx, web }object correctly.packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts (1)
41-45: LGTM: per-platform entries shape is correctThe test now passes the { all, lynx, web } structure expected by registerConsoleShortcuts. Looks good.
packages/rspeedy/plugin-qrcode/src/index.ts (1)
13-14: Fix middleware import path or ensure package exports/typesThe root import may fail type-resolution unless the package exposes a root export. Import the explicit Node entry, or add proper exports/types to the middleware package.
Apply:
-import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware' +import { createWebVirtualFilesMiddleware } from '@lynx-js/web-rsbuild-server-middleware/node'If you intend to keep the root path, ensure the middleware package provides an exports map and types.
Run to verify wiring:
#!/bin/bash set -euo pipefail fd -a package.json | rg -n 'packages/(rspeedy/plugin-qrcode|web-platform/web-rsbuild-server-middleware)/package.json' -N | cut -d: -f1 | while read -r p; do echo "==> $p"; cat "$p"; echo; done echo "Searching for exports['./node'] mapping..." rg -n '"exports"\s*:\s*{' -n -C3 packages/web-platform/web-rsbuild-server-middleware/package.json || true
| // <port> is supported in `dev.assetPrefix`, we should replace it with the real port | ||
| let base: string | ||
| try { | ||
| base = new URL( | ||
| '', | ||
| assetPrefix.replaceAll('<port>', String(port)), | ||
| ).toString() | ||
| } catch { | ||
| base = `http://localhost:${port}/` | ||
| } |
There was a problem hiding this comment.
Relative assetPrefix loses its path in fallback
If dev.assetPrefix is relative (e.g. "/assets"), the catch branch discards its path and falls back to just http://localhost:/. Resolve the (possibly relative) prefix against localhost to preserve the path.
Apply:
let base: string
try {
base = new URL(
'',
assetPrefix.replaceAll('<port>', String(port)),
).toString()
} catch {
- base = `http://localhost:${port}/`
+ // Preserve relative or path-only assetPrefix by resolving against localhost
+ base = new URL(
+ assetPrefix.replaceAll('<port>', String(port)),
+ `http://localhost:${port}/`,
+ ).toString()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // <port> is supported in `dev.assetPrefix`, we should replace it with the real port | |
| let base: string | |
| try { | |
| base = new URL( | |
| '', | |
| assetPrefix.replaceAll('<port>', String(port)), | |
| ).toString() | |
| } catch { | |
| base = `http://localhost:${port}/` | |
| } | |
| // <port> is supported in `dev.assetPrefix`, we should replace it with the real port | |
| let base: string | |
| try { | |
| base = new URL( | |
| '', | |
| assetPrefix.replaceAll('<port>', String(port)), | |
| ).toString() | |
| } catch { | |
| // Preserve relative or path-only assetPrefix by resolving against localhost | |
| base = new URL( | |
| assetPrefix.replaceAll('<port>', String(port)), | |
| `http://localhost:${port}/`, | |
| ).toString() | |
| } |
🤖 Prompt for AI Agents
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts around lines 38-47: the
catch branch currently falls back to "http://localhost:<port>/" and drops any
path from a relative dev.assetPrefix (e.g. "/assets"); replace the catch logic
to first substitute <port> in assetPrefix and then resolve that value against a
localhost base (e.g. use new URL(prefixWithPort, `http://localhost:${port}`)) so
both relative and absolute prefixes preserve their path when the initial
URL(...) call fails.
React Example#5817 Bundle Size — 237.56KiB (0%).4ed1eae(current) vs 8983d4c main#5785(baseline) Bundle metrics
|
| Current #5817 |
Baseline #5785 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
166 |
166 |
| Duplicate Modules |
68 |
68 |
| Duplicate Code |
46.82% |
46.82% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5817 |
Baseline #5785 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.8KiB |
91.8KiB |
Bundle analysis report Branch PupilTong:p/hw/web-qrcode-url Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (2)
71-73: Type inconsistency in function signature.The
devUrlsparameter has typeRecord<string, string | symbol>but based on thegenerateDevUrlsreturn type and usage, it should beRecord<string, string>. Thesymboltype appears to be legacy from cancel token handling that's no longer needed.Apply this diff to align types:
async function loop( options: Options, value: { lynx: string | symbol | undefined, web: string | undefined }, - devUrls: { lynx: Record<string, string | symbol>, web: string }, + devUrls: { lynx: Record<string, string>, web: string }, currentEntry: string, ) {
157-158: Type casting can be eliminated.The
as stringcast on Line 158 is unnecessary sincevalue.lynxis already typed correctly in the function signature, and the cast could hide potential undefined values.Apply this diff to remove unnecessary casting:
- await options.onPrint?.(value.lynx as string | undefined, value.web) - showQRCode(value.lynx as string, value.web) + await options.onPrint?.(value.lynx, value.web) + showQRCode(value.lynx, value.web)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
generateDevUrls(10-70)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-17)
⏰ 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). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (9)
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (4)
56-63: Fix URL query parameter encoding.The
outputPathnameis directly inserted into the query string without URL encoding, which can create malformed URLs when the pathname contains special characters like slashes, spaces, or query delimiters.When encoding a string to put in a URL component (a querystring parameter), you should call encodeURIComponent.
Apply this diff to encode the query parameter:
const web = new URL( - `/web?casename=${outputPathname}`, + `/web?casename=${encodeURIComponent(outputPathname)}`, base, ).toString()
38-48: Fix relative assetPrefix fallback handling.When
dev.assetPrefixis a relative path (e.g., "/assets"), the catch block's fallback tohttp://localhost:${port}/discards the path information. This can break the generated URLs when the application expects assets to be served from a specific path.Apply this diff to preserve path information:
let base: string try { base = new URL( '', assetPrefix.replaceAll('<port>', String(port)), ).toString() } catch { - base = `http://localhost:${port}/` + // Preserve relative or path-only assetPrefix by resolving against localhost + base = new URL( + assetPrefix.replaceAll('<port>', String(port)), + `http://localhost:${port}/`, + ).toString() }
15-15: LGTM! Return type correctly reflects dual-environment support.The updated return type
{ lynx: Record<string, string>, web: string }properly represents the dual-environment structure needed for web platform support.
56-69: Web URL generation logic is sound.The logic correctly:
- Computes the web pathname using the same base URL as lynx
- Constructs a proper web preview URL with the casename parameter
- Returns the structured object with both lynx and web URLs
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (5)
16-20: Well-structured entries interface for multi-platform support.The updated
entriesinterface withall,lynx, andwebarrays provides clear separation of concerns for different environments while maintaining backward compatibility through theallarray.
27-27: Improved onPrint callback signature.The updated signature
(lynx?: string, web?: string) => Promise<void>correctly handles dual URLs and optional parameters, matching the multi-platform architecture.
39-56: Robust entry selection and URL handling.The logic correctly:
- Prioritizes lynx entries as the default when available
- Properly gates URLs based on entry environment support
- Calls
onPrintandshowQRCodewith appropriate parameters
113-125: Efficient hint generation for entry selection.The implementation correctly generates hints by computing URLs for each entry option, with proper fallback logic (
lynx[currentSchema] ?? web) for display purposes.
170-186: Proper environment gating in getCurrentUrl.The function correctly:
- Checks if the current entry is supported in each environment
- Returns undefined URLs for unsupported environments
- Prevents displaying inappropriate QR codes (e.g., lynx QR for web-only entries)
There was a problem hiding this comment.
Like I said in https://github.com/lynx-family/lynx-stack/pull/1816/files#r2384891713. This change would make the QRCode too complex. And also would make it REQUIRED for Lynx development.
The QRCode plugin is only made for quickly display the URLs of .lynx.bundle that can be scanned with LynxExplorer. Since we already have a web-dev-middleware, I think we could use it for Web Platform development (and even Lynx development).
Here are some common use cases:
1. Use Rspeedy without qrcode-plugin
On the terminal:
We use server.printUrls to print the URL to the development page (without the ?casename query)
➜ Local: http://localhost:3000/lynx
➜ Local: http://localhost:3000/web
➜ Network: http://<host>:3000/lynx
➜ Network: http://<host>:3000/web
On the web page (http://<host>:<port>/web):
We display URLs for all the entries.
Or we could even create a <lynx-view> for each entry.
On the lynx page (http://<host>:<port>/lynx):
We display QRCode and URLs for all the entires.
2. Use Rspeedy with qrcode-plugin
there is nothing special, only the QRCode for the .lynx.bundle are displayed on the terminal:
● Scan with Lynx
│
◆ <qrcode>
│
◆ http://10.67.172.118:3000/main2.lynx.bundle?fullscreen=true
➜ Local: http://localhost:3000/lynx
➜ Local: http://localhost:3000/web
➜ Network: http://<host>:3000/lynx
➜ Network: http://<host>:3000/web
◆ Usage
│ r Switch entries
│ a Switch schema
│ h Help
│ q Quit
What do you think
| ) | ||
|
|
||
| return typeof customSchema === 'string' | ||
| const outputPathname = new URL( |
There was a problem hiding this comment.
We should respect the schemaFn option, even for Web.
| ]), | ||
| ] | ||
| if (allEntries.length === 0) { | ||
| // Not lynx or web environment, skip print |
There was a problem hiding this comment.
nitpick: the comment does not match the condition.
| const value: string | symbol = Object.values(devUrls)[0]! | ||
| await options.onPrint?.(value) | ||
| showQRCode(value) | ||
| const hasLynxEntry = options.entries.lynx.includes(currentEntry) |
There was a problem hiding this comment.
Actually, I don't think it's a good idea to have environments mixed-up with entries. It's too complex.
Can we only display the QRCode of Lynx in the terminal, with the http://<host>:<port>/web printed as the entrypoint of the whole Web Platform, on which page display links to each entry.
PupilTong
force-pushed
the
p/hw/web-qrcode-url
branch
2 times, most recently
from
3eba98d to
81dcb1e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
75-75: URL-encode the pathname in the query parameter.
pathnamecan contain slashes (e.g.,/dist/entry.web.bundle), which will break the query string structure. UseencodeURIComponentto properly encode it.Apply this diff:
- const url = new URL(`/web?casename=${pathname}`, base).toString() + const url = new URL(`/web?casename=${encodeURIComponent(pathname)}`, base).toString()Based on past review comments.
68-70: Preserve relative assetPrefix path in fallback.When
assetPublicPathis relative (e.g.,"/assets"),URL.parse()returnsnulland the fallback discards the path component, resulting inhttp://localhost:<port>/instead ofhttp://localhost:<port>/assets/.Apply this diff:
- const base = URL.parse(assetPublicPath) - ? assetPublicPath - : `http://localhost:${port}/` + const base = new URL( + assetPublicPath, + `http://localhost:${port}/`, + ).toString()Based on past review comments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/tricky-lamps-battle.md(1 hunks)examples/react/lynx.config.js(1 hunks)packages/rspeedy/plugin-qrcode/package.json(1 hunks)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts(2 hunks)packages/rspeedy/plugin-qrcode/src/index.ts(4 hunks)packages/rspeedy/plugin-qrcode/src/shortcuts.ts(9 hunks)packages/rspeedy/plugin-qrcode/src/showQRCode.ts(1 hunks)packages/rspeedy/plugin-qrcode/test/preview.test.ts(5 hunks)packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts(2 hunks)packages/rspeedy/plugin-qrcode/tsconfig.build.json(1 hunks)packages/rspeedy/plugin-qrcode/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/rspeedy/plugin-qrcode/test/shortcuts.test.ts
- examples/react/lynx.config.js
- .changeset/tricky-lamps-battle.md
- packages/rspeedy/plugin-qrcode/package.json
- packages/rspeedy/plugin-qrcode/tsconfig.json
- packages/rspeedy/plugin-qrcode/tsconfig.build.json
🧰 Additional context used
🧬 Code graph analysis (4)
packages/rspeedy/plugin-qrcode/src/index.ts (2)
packages/web-platform/web-rsbuild-server-middleware/src/node/index.ts (1)
createWebVirtualFilesMiddleware(22-89)packages/rspeedy/plugin-qrcode/src/shortcuts.ts (1)
registerConsoleShortcuts(32-75)
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
packages/rspeedy/core/src/index.ts (1)
RsbuildPluginAPI(67-67)packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
CustomizedSchemaFn(22-24)packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (2)
generateExplorerDevUrls(40-58)generateWebDevUrls(60-82)packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
showQRCode(8-22)
packages/rspeedy/plugin-qrcode/test/preview.test.ts (1)
packages/rspeedy/plugin-qrcode/src/index.ts (1)
pluginQRCode(93-171)
🔇 Additional comments (10)
packages/rspeedy/plugin-qrcode/src/generateDevUrls.ts (1)
60-82: Consider respecting schemaFn for web URLs.Currently,
generateWebDevUrlsgenerates plain URLs without applyingschemaFn. This creates an asymmetry withgenerateExplorerDevUrlswhich respects custom schemas. Consider whether web URLs should also support custom schema transformation for consistency.Based on past review comments.
packages/rspeedy/plugin-qrcode/src/showQRCode.ts (1)
8-22: LGTM!The refactored signature cleanly separates Lynx and web preview concerns, and the conditional rendering logic correctly handles both environments.
packages/rspeedy/plugin-qrcode/src/index.ts (3)
106-110: LGTM!The middleware registration correctly gates on the presence of the web environment and applies the middleware at the expected path.
147-156: LGTM!The refactored
mainfunction correctly handles both Lynx and web environments, and the early return condition accurately reflects the comment.
13-14: Note: Type resolution issue tracked in past comments.Past review comments flagged that the import of
createWebVirtualFilesMiddlewarefails type-checking due to missing exports/types in the source package. This appears to be a package configuration issue outside the scope of this file.If the type-check failure persists, verify that
packages/web-platform/web-rsbuild-server-middleware/package.jsonproperly exports the module and declares types.Based on past review comments.
packages/rspeedy/plugin-qrcode/test/preview.test.ts (2)
180-213: LGTM!Test correctly validates behavior when entry is empty with lynx environment present.
215-248: LGTM!Test correctly validates behavior when lynx environment is absent, ensuring QR code is not rendered.
packages/rspeedy/plugin-qrcode/src/shortcuts.ts (3)
32-74: LGTM!The initialization logic correctly generates both Lynx explorer URLs and web URLs, and properly invokes
onPrintandshowQRCodewith both parameters.
117-136: LGTM!Entry-switching logic correctly regenerates explorer URLs for hint display and updates the current value.
137-157: LGTM!Schema-switching logic correctly regenerates the URL map and updates the current schema/value.
PupilTong
force-pushed
the
p/hw/web-qrcode-url
branch
from
81dcb1e to
4ed1eae
Compare
2 participants
#1806
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
Checklist