feat: add playground native preview#2530
Conversation
🦋 Changeset detectedLatest commit: c6d2b1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 JSON-first globalProps support and runtime merging, an in-memory dev payload store and dev-bundle URL endpoint, QR/share UI and style updates, base64url utility refactor, a small TypeScript CSS shim, build/dev script tweak, and removes two legacy page modules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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✅ All modified and coverable lines are covered by tests. 📢 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/genui/a2ui-playground/src/render.tsx (1)
119-155:⚠️ Potential issue | 🟡 Minor
globalPropsstate is set once and never updated bypostMessage.
globalPropsis initialized from the URL query and stored withuseStatewithout a setter being used anywhere. OnINIT_LYNX_VIEWpostMessages (line 136), onlyinitDatais updated; the original queryglobalPropscontinues to take precedence in the effect at line 149.If the parent iframe ever posts a fresh
INIT_LYNX_VIEWafter the initial query already supplied aglobalProps, the lynx-view will keep the staleglobalPropsand ignore the freshly postedinitData. Likely fine in the common path (one or the other is used), but worth confirming this matches the intended UX. If not, drop the precedence on stale state by usingbuildGlobalPropsFromInitData(initData)wheneverinitDatacame from a postMessage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/render.tsx` around lines 119 - 155, The bug is that globalProps is created via const [globalProps] = useState(...) and never updated, so POSTed INIT_LYNX_VIEW messages (handled by handleMessage which calls setInitData) cannot override the initial query globalProps; update the component to use a mutable state for globalProps (const [globalProps, setGlobalProps] = useState(...)) and in handleMessage (the MessageEvent handler that detects isInitLynxViewMessage) also call setGlobalProps to either use any globalProps from the incoming message or to buildGlobalPropsFromInitData(e.data.data) (or null) so the effect that assigns lynxView.globalProps will prefer the fresh posted payload rather than the stale initial query value.packages/genui/a2ui-playground/src/pages/DemosPage.tsx (1)
178-183:⚠️ Potential issue | 🟠 MajorEffect at Line 178 will reset the user's scenario/edits when
rspeedyDevUrlresolves.
doRenderis wrapped inuseCallbackwithrspeedyDevUrlin its deps (Line 175). The Lynx dev URL is fetched asynchronously byuseRspeedyDevUrl, so its value flips from''to a real URL ~one render after mount. When that happens,doRender's identity changes, and this effect re-fires withALL_SCENARIOS[0]and its messages — overwriting any scenario the user has already selected viahandleSelectScenarioand any edits they made tocustomJsonin the meantime.Restrict the initial-render effect to mount only, or trigger the Lynx-dev-URL build separately so it doesn't piggyback on the "select first scenario" effect.
🐛 Suggested fix
- useEffect(() => { - if (ALL_SCENARIOS[0]) { - const json = formatJson(ALL_SCENARIOS[0].messages); - doRender(json, ALL_SCENARIOS[0]); - } - }, [doRender]); + useEffect(() => { + if (ALL_SCENARIOS[0]) { + const json = formatJson(ALL_SCENARIOS[0].messages); + doRender(json, ALL_SCENARIOS[0]); + } + // Run only on mount; re-running when `doRender` changes (e.g. after + // `rspeedyDevUrl` resolves) would overwrite the user's current scenario/JSON. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + // When the rspeedy dev URL resolves later, re-build the Lynx dev URL + // for whatever the user is currently viewing. + useEffect(() => { + if (rspeedyDevUrl) { + doRender(customJson, currentScenario); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [rspeedyDevUrl]);(Adjust to the project's preferred eslint-disable convention.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx` around lines 178 - 183, The effect that selects the initial scenario should run once on mount instead of depending on doRender (which changes when rspeedyDevUrl resolves) — change the useEffect([...], [doRender]) that references ALL_SCENARIOS and doRender to run only on mount by using an empty dependency array and suppressing the exhaustive-deps warning (e.g., add an eslint-disable-next-line react-hooks/exhaustive-deps above this useEffect). This prevents rspeedyDevUrl-driven identity changes to doRender from re-running the effect and overwriting user selections/edits (handleSelectScenario, customJson); keep doRender and its useCallback (with rspeedyDevUrl) intact so Lynx URL logic remains untouched.
🧹 Nitpick comments (4)
packages/genui/a2ui-playground/lynx-src/App.tsx (1)
36-61:parseJsonLikeStringfalls back to returning the raw string.When no parse attempt succeeds (after the initial parse and up to 3 URL-decode cycles), the function returns the original
inputstring. Downstream,normalizeInitDataLikehappily assigns this string toout.messages/out.actionMocks. The string then propagates intoloadMessages→normalizePayloadToMessages, which retriesJSON.parseonce more and silently returns[]on failure.Net effect: a malformed
messages/actionMocksvalue is silently swallowed with no error surfaced to the user, who only sees an empty UI. Consider returningundefined(or explicit error sentinel) fromparseJsonLikeStringon total failure so the caller can either skip the field or surface a parse error viasetError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/lynx-src/App.tsx` around lines 36 - 61, parseJsonLikeString currently returns the original raw input when all parse/URL-decode attempts fail, which lets malformed JSON silently propagate into normalizeInitDataLike → out.messages/out.actionMocks and ultimately be swallowed by normalizePayloadToMessages; change parseJsonLikeString to return undefined instead of the raw string on total failure, and update callers (notably normalizeInitDataLike and any code path into loadMessages/normalizePayloadToMessages) to treat undefined as “no valid JSON” — skip assigning the field and surface a parse error via setError (or another explicit sentinel) so malformed payloads are not silently converted to empty state.packages/genui/a2ui-playground/rsbuild.config.ts (1)
11-36: Best-effortfindLocalIpmay pick the wrong interface.
findLocalIpreturns the first non-internal IPv4 it encounters, iterating inObject.keys(networkInterfaces())order. On dev machines with multiple interfaces (Wi-Fi + Ethernet + VPN + Docker/WSL bridges) the chosen interface is essentially nondeterministic and frequently not the one a phone on the same Wi-Fi can reach, breaking the QR-code-to-device flow silently.Consider:
- Allowing override via env (e.g.
RSPEEDY_HOST/LYNX_DEV_HOST).- Falling back to
req.headers.host(stripping port) so the URL inherits whatever host the developer is already using to reach the dev server.♻️ Sketch using request host as fallback
- (middlewares) => { - middlewares.unshift((req, res, next) => { + (middlewares) => { + middlewares.unshift((req, res, next) => { if (req.url?.startsWith('/__rspeedy_url')) { - const url = buildRspeedyBundleUrl(); + const hostHeader = req.headers.host ?? ''; + const reqHost = hostHeader.split(':')[0] || findLocalIp(); + const url = `http://${reqHost}:${RSPEEDY_PORT}/main.lynx.js`; res.setHeader('Content-Type', 'application/json');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/rsbuild.config.ts` around lines 11 - 36, findLocalIp currently picks the first non-internal IPv4 and buildRspeedyBundleUrl always uses it, which can choose the wrong interface; change buildRspeedyBundleUrl to first check for an explicit host override via env vars (e.g. process.env.RSPEEDY_HOST or process.env.LYNX_DEV_HOST) and use that if present, otherwise accept an optional host parameter (e.g. host?: string) so callers can pass req.headers.host (strip any :port) as the preferred runtime fallback, and only if neither env nor caller-provided host exist call findLocalIp() and build the URL using that host + RSPEEDY_PORT/main.lynx.js (ensure you strip any port when using req.headers.host and preserve RSPEEDY_PORT when constructing the final URL).packages/genui/a2ui-playground/package.json (1)
9-9: Verify the dev workflow with the new native-preview flow.A couple of concerns about chaining
pnpm run build:lynx && rsbuild dev:
build:lynxproduces a static lynx bundle once at startup. Subsequent edits tolynx-src/**won't be reflected in the servedmain.lynx.jsuntildevis restarted, which makes web-iframe preview stale during Lynx development.- The new
/__rspeedy_urlmiddleware inrsbuild.config.tsreturnshttp://<ip>:3000/main.lynx.js, which assumes an rspeedy dev server is running on port 3000. Just runningpnpm devdoes not startdev:lynx, so the native preview QR/URL surfaced fromuseRspeedyDevUrlwill point at a port that is not actually serving anything unless the developer remembers to rundev:lynxin a second terminal.Consider one of:
- Running
dev:lynxandrsbuild devconcurrently (e.g. vianpm-run-all -p/concurrently) so both servers are live.- Updating
README/AGENTS.mdto document the two-process workflow explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/package.json` at line 9, The dev script chains "build:lynx && rsbuild dev" which builds a static main.lynx.js once and leaves the rsbuild dev server pointing to a port that may not actually serve the lynx bundle; update the "dev" script in package.json to run the lynx dev server and rsbuild dev concurrently (e.g., use npm-run-all -p or concurrently to start "dev:lynx" alongside "rsbuild dev") so edits under lynx-src/** are live, and also update rsbuild.config.ts middleware that returns "/__rspeedy_url" (and any consumer like useRspeedyDevUrl) or the README/AGENTS.md to clearly document the two-process requirement if you choose not to run both concurrently.packages/genui/a2ui-playground/src/pages/DemosPage.tsx (1)
34-51: The Clipboard API is the modern standard, but the suggested async refactor requires call site changes.
document.execCommand('copy')is deprecated in favor ofnavigator.clipboard.writeText(). However, the suggested refactor changes the return type frombooleantoPromise<boolean>, which would require theonClickhandler at line 360 to become async. The current handler is synchronous and immediately reads theokresult to conditionally update state. Consider either:
- Making the
onClickhandler async:onClick={async () => { const ok = await copyToClipboard(full); if (ok) { ... } }}- Or keeping a synchronous signature that attempts the modern API but returns immediately with a boolean (note:
navigator.clipboard.writeTextis inherently async, so this approach would need to fall back toexecCommandsynchronously).The lint concern mentioned in the code comment about
navigatorbeing treated as Node-only is not applicable here—the ESLint config explicitly providesglobals.browserfor this file context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx` around lines 34 - 51, The current copyToClipboard function uses deprecated document.execCommand and the refactor to navigator.clipboard.writeText must either propagate an async result or retain a synchronous signature; update copyToClipboard to be async (returning Promise<boolean>) and use navigator.clipboard.writeText(text) with try/catch returning true/false, then change the caller (the onClick handler that currently reads the boolean immediately) to be async and await copyToClipboard(full) before updating state, or alternatively keep copyToClipboard synchronous by attempting navigator.clipboard.writeText() when available but falling back to the existing execCommand path and still returning a boolean; locate the function copyToClipboard and the onClick handler that reads the result and apply one of these two fixes consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 202-211: The unconditional console.info calls inside the useEffect
(referencing didLogRef, useEffect, and the variables globalProps,
globalPropsData, rawInitData, initData, effectiveData) should be gated so they
only run in development; wrap the logging behind a dev/debug check (e.g.
import.meta.env.DEV or process.env.NODE_ENV !== 'production') or remove them
entirely for production builds, keeping didLogRef logic but only invoking
console.info when the dev flag is true to avoid leaking large or user-authored
payloads in production.
In `@packages/genui/a2ui-playground/rsbuild.config.ts`:
- Line 9: Default RSPEEDY_PORT is causing frontend to point at port 3000 while
rsbuild dev may claim that port; change the default in RSPEEDY_PORT (symbol:
RSPEEDY_PORT) from 3000 to 3100 in rsbuild.config.ts and also set server.port:
3100 in lynx.config.ts so the /__rspeedy_url endpoint and the running rspeedy
dev server use the same non-conflicting port; update any environment docs or dev
scripts that rely on RSPEEDY_PORT to reflect 3100.
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 138-170: The anonymous async IIFE that posts to
`${rspeedyDevUrl}/__a2ui_payload` can throw on network failure or during
res.json(); wrap the fetch + response parsing + subsequent URL mutation in a
try/catch, log or swallow the error, and return early on error so the code keeps
the original inline URL fallback; only proceed to mutate the URL and call
setLynxDevUrl(u.toString()) (and use lynxUrlSeqRef.current/seq check) when
fetch/res.json succeed and res.ok is true, and preserve existing checks for
typeof data.messagesUrl/data.actionMocksUrl and actionMocks before setting
search params.
In `@packages/genui/a2ui-playground/src/utils/renderUrl.ts`:
- Around line 17-21: The query now stores raw JSON via params.set('messages',
JSON.stringify(init.messages)) and params.set('actionMocks',
JSON.stringify(init.actionMocks)), which can blow up URL/QR length; change
renderUrl.ts to preserve compact base64url encoding for these payloads (or
implement a length-based fallback): when JSON.stringify(...) length (or
resulting encoded query length) exceeds a safe threshold (e.g., test with your
largest preset/QR limit), encode the payload using base64url and set a
flag/query param indicating base64 encoding, otherwise keep the plain JSON
encoding; update handling for both init.messages and init.actionMocks so the
consumer can detect and decode base64url when present (and verify with the
DemosPage.tsx QR flow).
---
Outside diff comments:
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 178-183: The effect that selects the initial scenario should run
once on mount instead of depending on doRender (which changes when rspeedyDevUrl
resolves) — change the useEffect([...], [doRender]) that references
ALL_SCENARIOS and doRender to run only on mount by using an empty dependency
array and suppressing the exhaustive-deps warning (e.g., add an
eslint-disable-next-line react-hooks/exhaustive-deps above this useEffect). This
prevents rspeedyDevUrl-driven identity changes to doRender from re-running the
effect and overwriting user selections/edits (handleSelectScenario, customJson);
keep doRender and its useCallback (with rspeedyDevUrl) intact so Lynx URL logic
remains untouched.
In `@packages/genui/a2ui-playground/src/render.tsx`:
- Around line 119-155: The bug is that globalProps is created via const
[globalProps] = useState(...) and never updated, so POSTed INIT_LYNX_VIEW
messages (handled by handleMessage which calls setInitData) cannot override the
initial query globalProps; update the component to use a mutable state for
globalProps (const [globalProps, setGlobalProps] = useState(...)) and in
handleMessage (the MessageEvent handler that detects isInitLynxViewMessage) also
call setGlobalProps to either use any globalProps from the incoming message or
to buildGlobalPropsFromInitData(e.data.data) (or null) so the effect that
assigns lynxView.globalProps will prefer the fresh posted payload rather than
the stale initial query value.
---
Nitpick comments:
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 36-61: parseJsonLikeString currently returns the original raw
input when all parse/URL-decode attempts fail, which lets malformed JSON
silently propagate into normalizeInitDataLike → out.messages/out.actionMocks and
ultimately be swallowed by normalizePayloadToMessages; change
parseJsonLikeString to return undefined instead of the raw string on total
failure, and update callers (notably normalizeInitDataLike and any code path
into loadMessages/normalizePayloadToMessages) to treat undefined as “no valid
JSON” — skip assigning the field and surface a parse error via setError (or
another explicit sentinel) so malformed payloads are not silently converted to
empty state.
In `@packages/genui/a2ui-playground/package.json`:
- Line 9: The dev script chains "build:lynx && rsbuild dev" which builds a
static main.lynx.js once and leaves the rsbuild dev server pointing to a port
that may not actually serve the lynx bundle; update the "dev" script in
package.json to run the lynx dev server and rsbuild dev concurrently (e.g., use
npm-run-all -p or concurrently to start "dev:lynx" alongside "rsbuild dev") so
edits under lynx-src/** are live, and also update rsbuild.config.ts middleware
that returns "/__rspeedy_url" (and any consumer like useRspeedyDevUrl) or the
README/AGENTS.md to clearly document the two-process requirement if you choose
not to run both concurrently.
In `@packages/genui/a2ui-playground/rsbuild.config.ts`:
- Around line 11-36: findLocalIp currently picks the first non-internal IPv4 and
buildRspeedyBundleUrl always uses it, which can choose the wrong interface;
change buildRspeedyBundleUrl to first check for an explicit host override via
env vars (e.g. process.env.RSPEEDY_HOST or process.env.LYNX_DEV_HOST) and use
that if present, otherwise accept an optional host parameter (e.g. host?:
string) so callers can pass req.headers.host (strip any :port) as the preferred
runtime fallback, and only if neither env nor caller-provided host exist call
findLocalIp() and build the URL using that host + RSPEEDY_PORT/main.lynx.js
(ensure you strip any port when using req.headers.host and preserve RSPEEDY_PORT
when constructing the final URL).
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 34-51: The current copyToClipboard function uses deprecated
document.execCommand and the refactor to navigator.clipboard.writeText must
either propagate an async result or retain a synchronous signature; update
copyToClipboard to be async (returning Promise<boolean>) and use
navigator.clipboard.writeText(text) with try/catch returning true/false, then
change the caller (the onClick handler that currently reads the boolean
immediately) to be async and await copyToClipboard(full) before updating state,
or alternatively keep copyToClipboard synchronous by attempting
navigator.clipboard.writeText() when available but falling back to the existing
execCommand path and still returning a boolean; locate the function
copyToClipboard and the onClick handler that reads the result and apply one of
these two fixes consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c03c5ba-d3ba-45a3-9b72-f292c6edb04c
📒 Files selected for processing (10)
.changeset/few-monkeys-juggle.mdpackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/src/css.d.tspackages/genui/a2ui-playground/src/pages/DemosPage.tsxpackages/genui/a2ui-playground/src/render.tsxpackages/genui/a2ui-playground/src/styles.csspackages/genui/a2ui-playground/src/utils/base64url.tspackages/genui/a2ui-playground/src/utils/renderUrl.ts
💤 Files with no reviewable changes (1)
- packages/genui/a2ui-playground/src/utils/base64url.ts
Merging this PR will improve performance by 11.8%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
44.7 ms | 40 ms | +11.8% |
Comparing Sherry-hue:feat/native-preview (c6d2b1e) with main (caadd3b)
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 MTF Example#844 Bundle Size — 196.58KiB (0%).c6d2b1e(current) vs cf6e21c main#841(baseline) Bundle metrics
|
| Current #844 |
Baseline #841 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
174 |
174 |
| Duplicate Modules |
66 |
66 |
| Duplicate Code |
44.04% |
44.04% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #844 |
Baseline #841 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
85.35KiB |
85.35KiB |
Bundle analysis report Branch Sherry-hue:feat/native-preview Project dashboard
Generated by RelativeCI Documentation Report issue
React External#827 Bundle Size — 680.41KiB (0%).c6d2b1e(current) vs cf6e21c main#824(baseline) Bundle metrics
|
| Current #827 |
Baseline #824 |
|
|---|---|---|
| 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 Sherry-hue:feat/native-preview Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#7712 Bundle Size — 225.43KiB (0%).c6d2b1e(current) vs cf6e21c main#7709(baseline) Bundle metrics
|
| Current #7712 |
Baseline #7709 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
180 |
180 |
| Duplicate Modules |
69 |
69 |
| Duplicate Code |
44.54% |
44.54% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7712 |
Baseline #7709 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
79.67KiB |
79.67KiB |
Bundle analysis report Branch Sherry-hue:feat/native-preview Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9285 Bundle Size — 900.02KiB (0%).c6d2b1e(current) vs cf6e21c main#9282(baseline) Bundle metrics
|
| Current #9285 |
Baseline #9282 |
|
|---|---|---|
| Initial JS |
44.46KiB |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
 Modules |
228(-0.44%) |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.28% |
27.28% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #9285 |
Baseline #9282 |
|
|---|---|---|
| JS |
495.88KiB |
495.88KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch Sherry-hue:feat/native-preview Project dashboard
Generated by RelativeCI Documentation Report issue
Sherry-hue
force-pushed
the
feat/native-preview
branch
from
dea9f4a to
27049c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/genui/a2ui-playground/src/components/QrCode.tsx (1)
31-64: IncludingonErrorChangein the effect deps is fragile for unstable callers.If a future consumer passes an inline
(e) => setX(e)(instead of the stablesetLynxDevQrErrorused today), this effect will re-run on every parent render, re-invokingtoDataURLfor the samevalueand emitting a transient''viaonErrorChange?.('')at line 34 before the next encode resolves. Stash the latest callback in a ref and drop it from the deps to make the component robust.♻️ Proposed refactor
-import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; @@ const { value, size = 144, onErrorChange } = props; const [src, setSrc] = useState<string>(''); const [error, setError] = useState<string>(''); + + const onErrorChangeRef = useRef(onErrorChange); + useEffect(() => { + onErrorChangeRef.current = onErrorChange; + }, [onErrorChange]); @@ useEffect(() => { let cancelled = false; setError(''); - onErrorChange?.(''); + onErrorChangeRef.current?.(''); @@ if (!cancelled) { setSrc(url); setError(''); - onErrorChange?.(''); + onErrorChangeRef.current?.(''); } } catch (e) { if (!cancelled) { setSrc(''); const msg = e instanceof Error ? e.message : 'Failed to encode QR code'; setError(msg); - onErrorChange?.(msg); + onErrorChangeRef.current?.(msg); } } })(); @@ return () => { cancelled = true; }; - }, [onErrorChange, options, value]); + }, [options, value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/components/QrCode.tsx` around lines 31 - 64, The effect is fragile because onErrorChange is in the dependency array; move the latest onErrorChange into a ref (e.g., errorCbRef) and use that ref inside the useEffect instead of the prop so the effect depends only on stable values (value, options) and not on unstable caller functions; specifically, create a ref updated whenever onErrorChange changes, call errorCbRef.current(...) in place of onErrorChange?.(...) inside the async block and the error path, and remove onErrorChange from the effect deps to avoid re-running toDataURL unnecessarily.packages/genui/a2ui-playground/lynx.config.ts (1)
21-31: BoundpayloadStoreand run GC on reads too.
gcPayloads()is only invoked from the POST branch (line 81), so if a long-lived dev session does many GETs and few POSTs, expired entries are not collected; conversely, a stream of POSTs within the TTL window can grow the map unboundedly. Consider also enforcing a soft max entry count to evict the oldest, and callinggcPayloads()on the GET branch as well.♻️ Sketch
+const MAX_PAYLOAD_ENTRIES = 256; + function gcPayloads(): void { const now = Date.now(); for (const [id, p] of payloadStore) { if (now - p.createdAt > PAYLOAD_TTL_MS) { payloadStore.delete(id); } } + // Drop oldest if over cap (Map preserves insertion order). + while (payloadStore.size > MAX_PAYLOAD_ENTRIES) { + const oldest = payloadStore.keys().next().value; + if (oldest === undefined) break; + payloadStore.delete(oldest); + } }…and call
gcPayloads()near the top of the GET handler as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/lynx.config.ts` around lines 21 - 31, The payload GC only runs on POST so expired entries can accumulate; update gcPayloads, payloadStore and request handlers to run GC on reads too and impose a soft max size eviction policy: 1) call gcPayloads() at the start of the GET handler (where the GET branch is handled) as well as in the POST branch, 2) make payloadStore and gcPayloads visible/shared where handlers run (ensure the Map payloadStore and function gcPayloads() remain module-scoped/bound so both branches can call them), and 3) augment gcPayloads() (or add a small helper) to enforce a soft max entry count (e.g., if payloadStore.size > MAX_ENTRIES, evict oldest entries by createdAt) so the Map cannot grow unboundedly; reference payloadStore, gcPayloads(), PAYLOAD_TTL_MS and the GET/POST handler code paths when making these changes.packages/genui/a2ui-playground/src/render.tsx (1)
118-155: Confirm intent: URLglobalPropspermanently shadows post-mountINIT_LYNX_VIEWupdates.
globalPropsis captured once into state with no setter (line 125), and the effect prefers it overbuildGlobalPropsFromInitData(initData)(lines 149-150). Consequently, when the URL containsglobalProps, any subsequentINIT_LYNX_VIEWpostMessage updatesinitData(and is forwarded tolynxView.initData) butlynxView.globalPropsremains pinned to the URL’s original value — so the newmessages/actionMockswon’t reach the Lynx app viaglobalProps. If postMessage updates are intended to refresh the payload, you’ll want to rebuildglobalPropsfrom the latestinitDatawhen no URL globalProps applies, or recompute on every effect run.While here: since
globalPropsnever changes after mount,useMemoreads more cleanly thanuseStatewith an unused setter.♻️ One possible shape (recompute every run; URL still wins on first render)
- const initial = useMemo(() => { - const initData = parseInitDataFromQuery(); - const globalProps = parseGlobalPropsFromQuery(); - return { initData, globalProps }; - }, []); - const [initData, setInitData] = useState<InitData | null>(initial.initData); - const [globalProps] = useState<Record<string, unknown> | null>( - initial.globalProps, - ); + const queryGlobalProps = useMemo(() => parseGlobalPropsFromQuery(), []); + const [initData, setInitData] = useState<InitData | null>( + () => parseInitDataFromQuery(), + ); @@ - lynxView.initData = initData ?? {}; - // Align with native: prefer `globalProps` as the channel for A2UI payload. - lynxView.globalProps = globalProps ?? buildGlobalPropsFromInitData(initData) - ?? {}; + lynxView.initData = initData ?? {}; + // Align with native: prefer `globalProps` as the channel for A2UI payload. + lynxView.globalProps = queryGlobalProps + ?? buildGlobalPropsFromInitData(initData) + ?? {}; @@ - }, [globalProps, initData]); + }, [queryGlobalProps, initData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/render.tsx` around lines 118 - 155, The code captures URL-derived globalProps once into state (globalProps from parseGlobalPropsFromQuery in Render) which permanently overrides later INIT_LYNX_VIEW postMessage updates; to fix, stop pinning it: compute a persistent urlGlobalProps once (e.g. useMemo or a ref from parseGlobalPropsFromQuery) but in the effect that writes to lynxView use urlGlobalProps ?? buildGlobalPropsFromInitData(initData) ?? {} so buildGlobalPropsFromInitData is evaluated on every initData change; specifically, replace the useState([globalProps]) usage with a useMemo or ref holding parseGlobalPropsFromQuery(), and update the effect that sets lynxView.globalProps to prefer that memoized url value only if present, otherwise recompute from buildGlobalPropsFromInitData(initData) so postMessage updates to setInitData propagate into lynxView.globalProps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-playground/lynx.config.ts`:
- Around line 33-47: In readJsonBody, add a max-body-size guard and optional
Content-Type check: read req.headers['content-type'] and if present and not
matching 'application/json' reject immediately; while buffering chunks track
cumulative byte length and if it exceeds a safe limit (e.g. 1_048_576 bytes)
stop reading, remove listeners and reject with a clear error (and destroy the
socket or call req.destroy()), ensuring you reject with an Error and clean up
'data'/'end'/'error' listeners to avoid leaks; keep the rest of the JSON parse
flow intact so resolve/reject behavior of readJsonBody remains unchanged.
In `@packages/genui/a2ui-playground/src/render.tsx`:
- Around line 77-92: In parseGlobalPropsFromQuery, currently any parsed value
with typeof === 'object' (including arrays) is returned as a Record, so update
the validation to reject arrays and non-plain objects: after parsing in
parseGlobalPropsFromQuery, ensure parsed is non-null, typeof parsed ===
'object', Array.isArray(parsed) is false, and optionally confirm it's a plain
object (e.g., Object.getPrototypeOf(parsed) === Object.prototype) before casting
and returning; otherwise return null.
---
Nitpick comments:
In `@packages/genui/a2ui-playground/lynx.config.ts`:
- Around line 21-31: The payload GC only runs on POST so expired entries can
accumulate; update gcPayloads, payloadStore and request handlers to run GC on
reads too and impose a soft max size eviction policy: 1) call gcPayloads() at
the start of the GET handler (where the GET branch is handled) as well as in the
POST branch, 2) make payloadStore and gcPayloads visible/shared where handlers
run (ensure the Map payloadStore and function gcPayloads() remain
module-scoped/bound so both branches can call them), and 3) augment gcPayloads()
(or add a small helper) to enforce a soft max entry count (e.g., if
payloadStore.size > MAX_ENTRIES, evict oldest entries by createdAt) so the Map
cannot grow unboundedly; reference payloadStore, gcPayloads(), PAYLOAD_TTL_MS
and the GET/POST handler code paths when making these changes.
In `@packages/genui/a2ui-playground/src/components/QrCode.tsx`:
- Around line 31-64: The effect is fragile because onErrorChange is in the
dependency array; move the latest onErrorChange into a ref (e.g., errorCbRef)
and use that ref inside the useEffect instead of the prop so the effect depends
only on stable values (value, options) and not on unstable caller functions;
specifically, create a ref updated whenever onErrorChange changes, call
errorCbRef.current(...) in place of onErrorChange?.(...) inside the async block
and the error path, and remove onErrorChange from the effect deps to avoid
re-running toDataURL unnecessarily.
In `@packages/genui/a2ui-playground/src/render.tsx`:
- Around line 118-155: The code captures URL-derived globalProps once into state
(globalProps from parseGlobalPropsFromQuery in Render) which permanently
overrides later INIT_LYNX_VIEW postMessage updates; to fix, stop pinning it:
compute a persistent urlGlobalProps once (e.g. useMemo or a ref from
parseGlobalPropsFromQuery) but in the effect that writes to lynxView use
urlGlobalProps ?? buildGlobalPropsFromInitData(initData) ?? {} so
buildGlobalPropsFromInitData is evaluated on every initData change;
specifically, replace the useState([globalProps]) usage with a useMemo or ref
holding parseGlobalPropsFromQuery(), and update the effect that sets
lynxView.globalProps to prefer that memoized url value only if present,
otherwise recompute from buildGlobalPropsFromInitData(initData) so postMessage
updates to setInitData propagate into lynxView.globalProps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d211c449-d0a6-4758-83da-2f9ac344fd03
📒 Files selected for processing (14)
.changeset/few-monkeys-juggle.mdpackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/src/components/QrCode.tsxpackages/genui/a2ui-playground/src/css.d.tspackages/genui/a2ui-playground/src/pages/DemosPage.tsxpackages/genui/a2ui-playground/src/pages/Dynamic.tsxpackages/genui/a2ui-playground/src/pages/Static.tsxpackages/genui/a2ui-playground/src/render.tsxpackages/genui/a2ui-playground/src/styles.csspackages/genui/a2ui-playground/src/utils/base64url.tspackages/genui/a2ui-playground/src/utils/renderUrl.ts
💤 Files with no reviewable changes (3)
- packages/genui/a2ui-playground/src/pages/Static.tsx
- packages/genui/a2ui-playground/src/utils/base64url.ts
- packages/genui/a2ui-playground/src/pages/Dynamic.tsx
✅ Files skipped from review due to trivial changes (3)
- .changeset/few-monkeys-juggle.md
- packages/genui/a2ui-playground/src/css.d.ts
- packages/genui/a2ui-playground/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/genui/a2ui-playground/src/utils/renderUrl.ts
- packages/genui/a2ui-playground/rsbuild.config.ts
- packages/genui/a2ui-playground/src/pages/DemosPage.tsx
- packages/genui/a2ui-playground/lynx-src/App.tsx
Sherry-hue
force-pushed
the
feat/native-preview
branch
from
27049c8 to
327be8c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/genui/a2ui-playground/rsbuild.config.ts (1)
9-9:⚠️ Potential issue | 🔴 Critical
PORTis reused for both the rsbuild server and the rspeedy bundle URL — they cannot share a port.
rsbuild dev(this config) andrspeedy dev(lynx.config.ts) run as separate processes/servers. Here,server.port: PORTbinds rsbuild toPORT(default3000), andbuildRspeedyBundleUrl()returnshttp://<ip>:${PORT}/main.lynx.js— pointing the QR code//__rspeedy_urlconsumer at the rsbuild port, not the rspeedy port. When both default to3000, rspeedy auto-increments to3001/3002, andmain.lynx.jswill 404 from rsbuild while the actual bundle lives on the rspeedy port.Use a distinct env var (e.g.,
RSPEEDY_PORT) for the bundle URL and ensurelynx.config.tsbinds rspeedy to that same value, so the URL handed out by/__rspeedy_urlactually resolves to the rspeedy dev server.🛠️ Suggested change
-const PORT = Number(process.env.PORT ?? 3000); +const PORT = Number(process.env.PORT ?? 3000); +const RSPEEDY_PORT = Number(process.env.RSPEEDY_PORT ?? 3100); @@ function buildRspeedyBundleUrl(): string { const ip = findLocalIp(); - return `http://${ip}:${PORT}/main.lynx.js`; + return `http://${ip}:${RSPEEDY_PORT}/main.lynx.js`; }And in
lynx.config.ts, setserver.port: Number(process.env.RSPEEDY_PORT ?? 3100).Also applies to: 33-36, 47-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/rsbuild.config.ts` at line 9, The rsbuild config reuses PORT for both its own server and the rspeedy bundle URL causing wrong porting; change the bundle URL to use a separate env var (e.g., RSPEEDY_PORT) instead of PORT and update buildRspeedyBundleUrl() to interpolate RSPEEDY_PORT, and then ensure lynx.config.ts sets server.port from the same RSPEEDY_PORT (Number(process.env.RSPEEDY_PORT ?? 3100)) so the URL returned by /__rspeedy_url points to the actual rspeedy dev server rather than the rsbuild port.
🧹 Nitpick comments (3)
packages/genui/a2ui-playground/src/styles.css (2)
423-440: Add a visible focus style for.previewQrCopyBtn.The copy button defines
:hoverbut no:focus-visiblestate. Keyboard users will not get a discernible focus indicator (the default UA outline may also be suppressed by surrounding styles in some browsers). Mirroring the hover treatment on:focus-visiblekeeps the button keyboard-accessible.♻️ Suggested addition
.previewQrCopyBtn:hover { color: var(--geist-foreground); border-color: var(--geist-border-hover); } + +.previewQrCopyBtn:focus-visible { + color: var(--geist-foreground); + border-color: var(--geist-border-hover); + outline: 2px solid var(--geist-foreground); + outline-offset: 2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/styles.css` around lines 423 - 440, The .previewQrCopyBtn currently styles hover but lacks a keyboard focus style; add a :focus-visible rule for .previewQrCopyBtn that mirrors the :hover treatment (e.g., set color to var(--geist-foreground) and border-color to var(--geist-border-hover)) so keyboard users get a clear visible indicator, and ensure any existing outline suppression is handled appropriately to keep the focus ring accessible when :focus-visible applies.
396-403: Remove unused.previewQrUrlclass.The
.previewQrUrlrule (line 396) is not referenced anywhere in the codebase. The markup in DemosPage.tsx uses only.previewQrUrlRowand.previewQrUrlTextfor the dev-bundle URL row. Remove this unused CSS rule.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/styles.css` around lines 396 - 403, Remove the unused CSS rule .previewQrUrl from packages/genui/a2ui-playground/src/styles.css; locate the .previewQrUrl block (the selector and its declarations) and delete it, leaving the existing .previewQrUrlRow and .previewQrUrlText rules intact—also quickly grep for the symbol previewQrUrl to confirm there are no remaining references before committing.packages/genui/a2ui-playground/rsbuild.config.ts (1)
15-21: Drop the unnecessary type assertion onlist.
@types/node(v24.10.13) typesnetworkInterfaces()as returningNodeJS.Dict<NetworkInterfaceInfo[]>, whereNetworkInterfaceInfois a discriminated union withfamily: "IPv4" | "IPv6"as literal string types. The inlineas Array<{ ... }>cast is redundant, widens thefamilytype tostring | number, and forces thetypeof net.family === 'string'check that becomes unnecessary with proper typing.♻️ Proposed simplification
- for ( - const net of list as Array<{ - address: string; - family: string | number; - internal: boolean; - }> - ) { - const family = typeof net.family === 'string' - ? net.family - : `IPv${net.family}`; - if (family === 'IPv4' && !net.internal) { - return net.address; - } - } + for (const net of list) { + if (net.family === 'IPv4' && !net.internal) { + return net.address; + } + }
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/genui/a2ui-playground/rsbuild.config.ts`:
- Line 9: The rsbuild config reuses PORT for both its own server and the rspeedy
bundle URL causing wrong porting; change the bundle URL to use a separate env
var (e.g., RSPEEDY_PORT) instead of PORT and update buildRspeedyBundleUrl() to
interpolate RSPEEDY_PORT, and then ensure lynx.config.ts sets server.port from
the same RSPEEDY_PORT (Number(process.env.RSPEEDY_PORT ?? 3100)) so the URL
returned by /__rspeedy_url points to the actual rspeedy dev server rather than
the rsbuild port.
---
Nitpick comments:
In `@packages/genui/a2ui-playground/src/styles.css`:
- Around line 423-440: The .previewQrCopyBtn currently styles hover but lacks a
keyboard focus style; add a :focus-visible rule for .previewQrCopyBtn that
mirrors the :hover treatment (e.g., set color to var(--geist-foreground) and
border-color to var(--geist-border-hover)) so keyboard users get a clear visible
indicator, and ensure any existing outline suppression is handled appropriately
to keep the focus ring accessible when :focus-visible applies.
- Around line 396-403: Remove the unused CSS rule .previewQrUrl from
packages/genui/a2ui-playground/src/styles.css; locate the .previewQrUrl block
(the selector and its declarations) and delete it, leaving the existing
.previewQrUrlRow and .previewQrUrlText rules intact—also quickly grep for the
symbol previewQrUrl to confirm there are no remaining references before
committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 465a6bda-6978-4690-a0bc-1dd952dbe26d
📒 Files selected for processing (14)
.changeset/few-monkeys-juggle.mdpackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/src/components/QrCode.tsxpackages/genui/a2ui-playground/src/css.d.tspackages/genui/a2ui-playground/src/pages/DemosPage.tsxpackages/genui/a2ui-playground/src/pages/Dynamic.tsxpackages/genui/a2ui-playground/src/pages/Static.tsxpackages/genui/a2ui-playground/src/render.tsxpackages/genui/a2ui-playground/src/styles.csspackages/genui/a2ui-playground/src/utils/base64url.tspackages/genui/a2ui-playground/src/utils/renderUrl.ts
💤 Files with no reviewable changes (2)
- packages/genui/a2ui-playground/src/pages/Static.tsx
- packages/genui/a2ui-playground/src/pages/Dynamic.tsx
✅ Files skipped from review due to trivial changes (3)
- packages/genui/a2ui-playground/src/utils/renderUrl.ts
- packages/genui/a2ui-playground/src/css.d.ts
- .changeset/few-monkeys-juggle.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/genui/a2ui-playground/package.json
- packages/genui/a2ui-playground/lynx-src/App.tsx
- packages/genui/a2ui-playground/src/components/QrCode.tsx
- packages/genui/a2ui-playground/lynx.config.ts
- packages/genui/a2ui-playground/src/pages/DemosPage.tsx
Huxpro
left a comment
Huxpro
left a comment
There was a problem hiding this comment.
Review
1. __rspeedy_url returns the wrong port when the configured port is already in use
rsbuild.config.ts:27:
const PORT = Number(process.env.PORT ?? 3000);This is evaluated at module load time, but rsbuild auto-increments the port if it's already taken (e.g. 3000 → 3001). The /__rspeedy_url endpoint then returns a URL with port 3000 while the server is actually on 3001 — native preview will fail to connect.
The actual listening port should be read from the server instance at request time rather than from the static config value.
2. Dead didLogRef effect in App.tsx
lynx-src/App.tsx:120-124:
const didLogRef = useRef(false);
useEffect(() => {
if (didLogRef.current) return;
didLogRef.current = true;
}, [effectiveData, globalProps, globalPropsData, initData, rawInitData]);This sets a flag once and does nothing else — looks like debug logging was removed but the scaffolding was left behind. Should be deleted.
3. copyToClipboard uses deprecated document.execCommand('copy')
DemosPage.tsx:31-47: The comment says "some lint configs treat navigator as a Node-only API" but this code only runs in the browser. navigator.clipboard.writeText() is the standard API supported in all major browsers. document.execCommand('copy') is deprecated and will eventually be removed.
async function copyToClipboard(text: string): Promise<boolean> {
try {
await navigator.clipboard.writeText(text);
return true;
} catch {
return false;
}
}
Sherry-hue
force-pushed
the
feat/native-preview
branch
from
205fd2f to
8cc78a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/genui/a2ui-playground/src/pages/DemosPage.tsx (1)
203-208:⚠️ Potential issue | 🟠 MajorDon't reset the preview to the first preset when
rspeedyDevUrlarrives.Because
doRenderdepends onrspeedyDevUrl, this effect reruns after/__rspeedy_urlresolves and re-rendersALL_SCENARIOS[0]. If the user already picked another scenario or edited the JSON, their current preview gets overwritten. Make the bootstrap render one-shot, or rerender fromcustomJson/currentScenariowhen only the dev-bundle URL changes.Based on learnings: The a2ui-playground package must support two build targets:
webviarsbuild/core(React DOM preview) andlynxvialynx-js/rspeedy(Lynx preview).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx` around lines 203 - 208, The effect that calls doRender currently reruns when doRender (and transitively rspeedyDevUrl) changes, which resets the preview to ALL_SCENARIOS[0]; change the effect so the bootstrap render is one-shot or only runs when there is no user state: update the useEffect containing ALL_SCENARIOS[0] to run on mount (empty deps) or add a guard that only calls doRender when currentScenario is null/undefined and customJson is empty (i.e., user hasn't selected/edited), referencing the existing useEffect, ALL_SCENARIOS, doRender, rspeedyDevUrl, customJson and currentScenario to locate and implement the guard. Ensure subsequent changes to rspeedyDevUrl do not force re-render of the initially chosen scenario unless user state indicates no prior selection.
🧹 Nitpick comments (1)
packages/genui/a2ui-playground/lynx-src/App.tsx (1)
202-206: Remove the emptydidLogRefeffect.This hook no longer gates any logging or other side effect, so it only adds bootstrap noise and an unnecessary dependency list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/lynx-src/App.tsx` around lines 202 - 206, Remove the unused didLogRef and its accompanying useEffect: the const didLogRef = useRef(false) and the useEffect(...) block that checks/sets didLogRef.current should be deleted since it no longer gates any logging or side effects and only adds an unnecessary dependency list (effectiveData, globalProps, globalPropsData, initData, rawInitData); ensure no other code references didLogRef before removing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 71-88: normalizeInitDataLike currently assigns messagesUrl and
actionMocksUrl directly, causing percent-encoded query params to remain encoded;
update normalizeInitDataLike to run messagesUrl and actionMocksUrl through a
decoding helper (e.g., decodeQueryParamValue that implements the same multi-pass
percent-decoding loop used by parseJsonLikeString) before assigning to
out.messagesUrl and out.actionMocksUrl so loadMessages() and loadActionMocks()
receive decoded URLs; keep the JSON-specific parsing steps out (only apply the
decoding loop).
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 34-50: The temporary textarea created in copyToClipboard can leak
if document.execCommand throws; refactor copyToClipboard so the textarea
variable (ta) is declared outside the try block and removal
(document.body.removeChild(ta)) is moved into a finally block that runs whether
copy succeeds or fails; ensure you only attempt removal if ta was appended
(non-null) to avoid additional errors, and keep the function returning the copy
result (ok) or false on error.
- Around line 237-242: handleClear currently clears UI state but doesn't
invalidate any in-flight short-URL work; update handleClear to advance the
request sequence and clear the dev URL so any pending POST /__a2ui_payload
handlers fail the sequence check and cannot repopulate old QR/link state: inside
handleClear, increment or otherwise change lynxUrlSeqRef.current (e.g., ++ or
set to a new token) and call the setter to reset lynxDevUrl (e.g.,
setLynxDevUrl('')) in addition to the existing setCustomJson, setRenderUrl,
setRenderQrError, and setError calls.
---
Outside diff comments:
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 203-208: The effect that calls doRender currently reruns when
doRender (and transitively rspeedyDevUrl) changes, which resets the preview to
ALL_SCENARIOS[0]; change the effect so the bootstrap render is one-shot or only
runs when there is no user state: update the useEffect containing
ALL_SCENARIOS[0] to run on mount (empty deps) or add a guard that only calls
doRender when currentScenario is null/undefined and customJson is empty (i.e.,
user hasn't selected/edited), referencing the existing useEffect, ALL_SCENARIOS,
doRender, rspeedyDevUrl, customJson and currentScenario to locate and implement
the guard. Ensure subsequent changes to rspeedyDevUrl do not force re-render of
the initially chosen scenario unless user state indicates no prior selection.
---
Nitpick comments:
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 202-206: Remove the unused didLogRef and its accompanying
useEffect: the const didLogRef = useRef(false) and the useEffect(...) block that
checks/sets didLogRef.current should be deleted since it no longer gates any
logging or side effects and only adds an unnecessary dependency list
(effectiveData, globalProps, globalPropsData, initData, rawInitData); ensure no
other code references didLogRef before removing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f80d6f39-6849-4ad4-adad-9b1722d4c851
📒 Files selected for processing (14)
.changeset/few-monkeys-juggle.mdpackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/src/components/QrCode.tsxpackages/genui/a2ui-playground/src/css.d.tspackages/genui/a2ui-playground/src/pages/DemosPage.tsxpackages/genui/a2ui-playground/src/pages/Dynamic.tsxpackages/genui/a2ui-playground/src/pages/Static.tsxpackages/genui/a2ui-playground/src/render.tsxpackages/genui/a2ui-playground/src/styles.csspackages/genui/a2ui-playground/src/utils/base64url.tspackages/genui/a2ui-playground/src/utils/renderUrl.ts
💤 Files with no reviewable changes (2)
- packages/genui/a2ui-playground/src/pages/Static.tsx
- packages/genui/a2ui-playground/src/pages/Dynamic.tsx
✅ Files skipped from review due to trivial changes (4)
- .changeset/few-monkeys-juggle.md
- packages/genui/a2ui-playground/src/utils/renderUrl.ts
- packages/genui/a2ui-playground/src/css.d.ts
- packages/genui/a2ui-playground/rsbuild.config.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/genui/a2ui-playground/package.json
- packages/genui/a2ui-playground/src/styles.css
- packages/genui/a2ui-playground/lynx.config.ts
- packages/genui/a2ui-playground/src/render.tsx
- packages/genui/a2ui-playground/src/components/QrCode.tsx
Sherry-hue
force-pushed
the
feat/native-preview
branch
2 times, most recently
from
9867416 to
ae5bc98
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/genui/a2ui-playground/src/pages/DemosPage.tsx (2)
34-50:⚠️ Potential issue | 🟡 MinorAlways remove the temporary textarea.
If
document.execCommand('copy')throws after append, the hidden textarea is never removed. Move cleanup into afinally(or useta.remove()) so repeated copy failures do not leak DOM nodes.Possible fix
function copyToClipboard(text: string): boolean { // Prefer a compatibility path here: some lint configs treat `navigator` as a Node-only API. + const ta = document.createElement('textarea'); try { - const ta = document.createElement('textarea'); ta.value = text; ta.style.position = 'fixed'; ta.style.left = '-9999px'; @@ ta.focus(); ta.select(); const ok = document.execCommand('copy'); - document.body.removeChild(ta); return ok; } catch { return false; + } finally { + ta.remove(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx` around lines 34 - 50, The copyToClipboard function can leak the temporary textarea if document.execCommand('copy') throws; ensure the created element (ta) is always removed by moving the cleanup into a finally block (or calling ta.remove() in finally) so document.body.removeChild(ta) runs regardless of success or exception; update the function body around the try/catch to guarantee ta is removed even on errors while preserving the returned boolean from execCommand.
237-242:⚠️ Potential issue | 🟠 MajorClear should also invalidate the Lynx preview state.
This only resets the web preview. The old
lynxDevUrl/copy state stays visible, and any in-flightPOST /__a2ui_payloadcan still pass the current sequence check and repopulate stale QR data after Clear.Possible fix
const handleClear = useCallback(() => { + lynxUrlSeqRef.current += 1; setCustomJson('[]'); setRenderUrl(''); + setLynxDevUrl(''); setRenderQrError(''); + setLynxDevQrError(''); + setLynxDevCopied(false); setError(''); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx` around lines 237 - 242, handleClear currently only resets web preview state (setCustomJson, setRenderUrl, setRenderQrError, setError) but leaves Lynx preview/copy and the payload sequence intact; update handleClear to also clear Lynx-specific state (e.g., call the setter for lynxDevUrl and any copy flag like setLynxCopied) and invalidate the in-flight POST acceptance token by bumping the payload sequence (e.g., setPayloadSequence(s => s + 1) or setPayloadSequence(Date.now())) so any late POST /__a2ui_payload responses are ignored and stale QR data cannot repopulate the UI.packages/genui/a2ui-playground/lynx.config.ts (1)
33-47:⚠️ Potential issue | 🟡 MinorBound the buffered request body.
readJsonBodystill buffers the entire request with no size or content-type guard. A single oversized or never-endingPOST /__a2ui_payloadcan pin memory or stall the dev server; please reject non-JSON bodies early and cap the buffered bytes before concatenating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/lynx.config.ts` around lines 33 - 47, readJsonBody currently buffers the entire request with no content-type or size checks; change readJsonBody(IncomingMessage) to first validate the Content-Type header is application/json (reject early if not) and then track accumulated bytes in the 'data' handler, rejecting the promise if a configurable MAX_BODY_BYTES (e.g. 1MB) is exceeded before concatenating; on resolve or reject remove all listeners ('data','end','error') to avoid leaks and ensure errors are wrapped as Error instances as before.packages/genui/a2ui-playground/lynx-src/App.tsx (1)
71-75:⚠️ Potential issue | 🟠 MajorDecode
messagesUrlandactionMocksUrlbefore storing them.
messagesandactionMocksalready go through the multi-pass decode path, but the URL fields are copied verbatim.DemosPagenow sends these through query params on the Lynx dev URL, so nativeglobalPropscan leave them percent-encoded andfetch()ends up targeting the wrong string. Apply the same decode loop to these URL values before assigning them intoout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui-playground/lynx-src/App.tsx` around lines 71 - 75, The URL fields messagesUrl and actionMocksUrl are copied verbatim into out but must be percent-decoded just like messages and actionMocks; locate the multi-pass decode logic used for messages/actionMocks and run messagesUrl and actionMocksUrl through that same decode loop before assigning to out.messagesUrl and out.actionMocksUrl so percent-encoded query params are correctly decoded for fetch().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 99-100: The component currently discards the render QR error state
(const [, setRenderQrError]) so failures are recorded but never shown; change
this to keep the state value (e.g., const [renderQrError, setRenderQrError]) and
use it (alongside lynxDevQrError / setLynxDevQrError) to render a visible
fallback in the "View on Device" block whenever QrCode returns null or encoding
fails: set the error message inside the QrCode-creation/encode catch path and
render that message (or an explanatory placeholder/button) in place of the blank
area so users see why the QR couldn’t be generated. Ensure the same pattern is
applied to the other instance noted (lines ~352-358) where a state setter was
used without the corresponding value.
---
Duplicate comments:
In `@packages/genui/a2ui-playground/lynx-src/App.tsx`:
- Around line 71-75: The URL fields messagesUrl and actionMocksUrl are copied
verbatim into out but must be percent-decoded just like messages and
actionMocks; locate the multi-pass decode logic used for messages/actionMocks
and run messagesUrl and actionMocksUrl through that same decode loop before
assigning to out.messagesUrl and out.actionMocksUrl so percent-encoded query
params are correctly decoded for fetch().
In `@packages/genui/a2ui-playground/lynx.config.ts`:
- Around line 33-47: readJsonBody currently buffers the entire request with no
content-type or size checks; change readJsonBody(IncomingMessage) to first
validate the Content-Type header is application/json (reject early if not) and
then track accumulated bytes in the 'data' handler, rejecting the promise if a
configurable MAX_BODY_BYTES (e.g. 1MB) is exceeded before concatenating; on
resolve or reject remove all listeners ('data','end','error') to avoid leaks and
ensure errors are wrapped as Error instances as before.
In `@packages/genui/a2ui-playground/src/pages/DemosPage.tsx`:
- Around line 34-50: The copyToClipboard function can leak the temporary
textarea if document.execCommand('copy') throws; ensure the created element (ta)
is always removed by moving the cleanup into a finally block (or calling
ta.remove() in finally) so document.body.removeChild(ta) runs regardless of
success or exception; update the function body around the try/catch to guarantee
ta is removed even on errors while preserving the returned boolean from
execCommand.
- Around line 237-242: handleClear currently only resets web preview state
(setCustomJson, setRenderUrl, setRenderQrError, setError) but leaves Lynx
preview/copy and the payload sequence intact; update handleClear to also clear
Lynx-specific state (e.g., call the setter for lynxDevUrl and any copy flag like
setLynxCopied) and invalidate the in-flight POST acceptance token by bumping the
payload sequence (e.g., setPayloadSequence(s => s + 1) or
setPayloadSequence(Date.now())) so any late POST /__a2ui_payload responses are
ignored and stale QR data cannot repopulate the UI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 28b01662-b86b-4130-aea7-3dac40e82a2e
📒 Files selected for processing (14)
.changeset/few-monkeys-juggle.mdpackages/genui/a2ui-playground/lynx-src/App.tsxpackages/genui/a2ui-playground/lynx.config.tspackages/genui/a2ui-playground/package.jsonpackages/genui/a2ui-playground/rsbuild.config.tspackages/genui/a2ui-playground/src/components/QrCode.tsxpackages/genui/a2ui-playground/src/css.d.tspackages/genui/a2ui-playground/src/pages/DemosPage.tsxpackages/genui/a2ui-playground/src/pages/Dynamic.tsxpackages/genui/a2ui-playground/src/pages/Static.tsxpackages/genui/a2ui-playground/src/render.tsxpackages/genui/a2ui-playground/src/styles.csspackages/genui/a2ui-playground/src/utils/base64url.tspackages/genui/a2ui-playground/src/utils/renderUrl.ts
💤 Files with no reviewable changes (2)
- packages/genui/a2ui-playground/src/pages/Dynamic.tsx
- packages/genui/a2ui-playground/src/pages/Static.tsx
✅ Files skipped from review due to trivial changes (4)
- .changeset/few-monkeys-juggle.md
- packages/genui/a2ui-playground/src/utils/renderUrl.ts
- packages/genui/a2ui-playground/src/css.d.ts
- packages/genui/a2ui-playground/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/genui/a2ui-playground/package.json
- packages/genui/a2ui-playground/rsbuild.config.ts
- packages/genui/a2ui-playground/src/render.tsx
Sherry-hue
force-pushed
the
feat/native-preview
branch
from
ae5bc98 to
a814b39
Compare
Sherry-hue
force-pushed
the
feat/native-preview
branch
from
a814b39 to
c6d2b1e
Compare
grate! These issues have been fixed. |
3 participants
Summary by CodeRabbit
New Features
Improvements
Chores
Removals
Checklist
Add a native preview QR code and pass the data to A2UIRender via globalProps: