ci(benchmark/react): enable fuzz benchmark#1716
Conversation
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a deterministic JSX generator, a rspeedy plugin to inline generated TSX, integrates the plugin into the React benchmark config, introduces two fuzz benchmark cases and scripts, updates TS config and typings for the macro, and adds a CLI helper and a devDependency. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 5
🧹 Nitpick comments (2)
benchmark/react/plugins/gen.ts (2)
341-357: Potential tight loop if attrs collide; cap attemptsIf a duplicate key is generated repeatedly (e.g., multiple
id), the loop can spin. Cap attempts to keep forward progress deterministic.- for (let i = 0; i < attrCount;) { + for (let i = 0, attempts = 0; i < attrCount && attempts < attrCount * 3; attempts++) { const attr = genAttr(r); if (attrKeys.has(attr.key)) { - continue; + continue; } - i++; + i++; code += ` ${attr.code}`;
12-20: Avoid patching global Math.random; inject RNG if possibleTemporarily overriding
Math.randomis fragile in shared plugin pipelines. Prefer passing a PRNG tohuman-idif supported, or wrap its output without touching globals. If keeping this approach, document the rationale.
📜 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 (8)
benchmark/react/cases/006-fuzz-complex-100/index.tsx(1 hunks)benchmark/react/cases/006-fuzz-complex-200/index.tsx(1 hunks)benchmark/react/lynx.config.ts(2 hunks)benchmark/react/package.json(3 hunks)benchmark/react/plugins/gen.ts(1 hunks)benchmark/react/plugins/pluginGenJSX.mjs(1 hunks)benchmark/react/rspeedy-env.d.ts(1 hunks)benchmark/react/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- benchmark/react/cases/006-fuzz-complex-200/index.tsx
- benchmark/react/tsconfig.json
- benchmark/react/cases/006-fuzz-complex-100/index.tsx
- benchmark/react/rspeedy-env.d.ts
- benchmark/react/lynx.config.ts
- benchmark/react/package.json
- benchmark/react/plugins/pluginGenJSX.mjs
⏰ 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). (1)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (2)
benchmark/react/plugins/gen.ts (2)
61-69: Weighting trick looks fineDuplicating 'Attr' in the pool to bias towards plain attributes is a simple and effective approach here.
192-209: Ignore suggestion to replace colon with hyphen
Your codebase uses TypeScript 5.9.2, which supports namespaced JSX attributes (e.g.<Foo a:b="x" />) as of TS 5.1, somain-thread:${eventType}is valid TSX. No change needed.Likely an incorrect or invalid review comment.
React Example#5126 Bundle Size — 238.9KiB (0%).8103d4e(current) vs dc28178 main#5125(baseline) Bundle metrics
|
| Current #5126 |
Baseline #5125 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
163 |
163 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.79% |
46.79% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5126 |
Baseline #5125 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
93.14KiB |
93.14KiB |
Bundle analysis report Branch hzy:p/hzy/bench_11 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5119 Bundle Size — 362.79KiB (0%).8103d4e(current) vs dc28178 main#5118(baseline) Bundle metrics
|
| Current #5119 |
Baseline #5118 |
|
|---|---|---|
| Initial JS |
145.21KiB |
145.21KiB |
| Initial CSS |
31.89KiB |
31.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
219(-0.45%) |
220 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.33% |
3.33% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5119 |
Baseline #5118 |
|
|---|---|---|
| JS |
236.88KiB |
236.88KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch hzy:p/hzy/bench_11 Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1716 will not alter performanceComparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
benchmark/react/plugins/gen.ts (5)
35-43: Sanitize generated identifiers; current output can be invalid JS (hyphens, leading digits). Ensure component-friendly casing.human-id may emit hyphens and leading digits; these break identifiers and JSX component names.
Apply this diff and add a sanitizer:
const genJsIdentifier = (r: Random, prefix = 'value') => { const oldRandom = Math.random; try { Math.random = () => r.random(); - return `${prefix}${humanId({ adjectiveCount: 1 })}`; + const raw = `${prefix}${humanId({ adjectiveCount: 1 })}`; + return sanitizeIdentifier(raw); } finally { Math.random = oldRandom; } };Add this helper near the utilities (outside the selected range):
// Ensures a valid JS identifier and biases to start with a letter (Component-friendly). const sanitizeIdentifier = (s: string): string => { let id = s.replace(/[^A-Za-z0-9_$]/g, '_'); if (/^[0-9]/.test(id)) id = `v_${id}`; if (!/^[A-Z]/.test(id)) id = id.replace(/^./, c => c.toUpperCase()); return id; };
241-246: Ensure component names start with uppercase to avoid being treated as intrinsic elements.- const fcName = genJsIdentifier(r, ''); + const fcName = genJsIdentifier(r, 'C');
258-262: De-duplicate merged ref vars before splitting; duplicates cause duplicate props/state ids.- const [refVars, refStates] = divideRandomly(r, [ - ...jsx1.refVars, - ...jsx2.refVars, - ], COMPONENT_PROPS_RATE); + const merged = [...jsx1.refVars, ...jsx2.refVars]; + const seen = new Set<string>(); + const uniq = merged.filter(v => (seen.has(v.id) ? false : (seen.add(v.id), true))); + const [refVars, refStates] = divideRandomly(r, uniq, COMPONENT_PROPS_RATE);
288-293: Same de-dup required when there’s no children; avoid duplicate prop/state ids.- const [refVars, refStates] = divideRandomly( - r, - jsx.refVars, - COMPONENT_PROPS_RATE, - ); + const seen = new Set<string>(); + const uniq = jsx.refVars.filter(v => (seen.has(v.id) ? false : (seen.add(v.id), true))); + const [refVars, refStates] = divideRandomly(r, uniq, COMPONENT_PROPS_RATE);
445-449: Initialize refs with typed values and de-duplicate declarations/vars at the top-level.- return `(({useState}) => { -${g.decls.map(d => d.code).join('\n')} -${g.refVars.map(v => `const ${v.id} = "${genStr(r)}";`).join('\n')} -return () => (${g.code}); -})`; + // De-dupe decls by id and ref vars by identifier. + const declMap = new Map<string, string>(); + for (const d of g.decls) if (!declMap.has(d.id)) declMap.set(d.id, d.code); + const seen = new Set<string>(); + const uniqRefVars = g.refVars.filter(v => (seen.has(v.id) ? false : (seen.add(v.id), true))); + + return `(({useState}) => { +${[...declMap.values()].join('\n')} +${uniqRefVars.map(v => `const ${v.id} = ${genValue(r, v)};`).join('\n')} +return () => (${g.code}); +})`;
🧹 Nitpick comments (3)
benchmark/react/plugins/gen.ts (1)
205-206: Arrow handler in MT_Event returns undefined; inline expression form is clearer.- code: `${key}={() => { 'main-thread' }}`, + code: `${key}={() => 'main-thread'}`,benchmark/react/scripts/tryGen.ts (1)
11-18: Make the regex more permissive (whitespace, negative seeds) and avoid magic literal duplication.-console.log( - readFileSync(0).toString().replace( - /__GENERATE_JSX__\((\d+), ?(\d+)\)/g, - (_, $1: string, $2: string) => { - return gen(parseInt($1, 10), parseInt($2, 10)); - }, - ), -); +const re = /__GENERATE_JSX__\(\s*(-?\d+)\s*,\s*(\d+)\s*\)/g; +console.log( + readFileSync(0, 'utf8').replace(re, (_: string, $1: string, $2: string) => + gen(parseInt($1, 10), parseInt($2, 10)), + ), +);benchmark/react/plugins/pluginGenJSX.mjs (1)
18-33: Short-circuit when there’s no macro; reduces work and preserves sourcemaps for untouched files.- api.transform({}, (context) => { - const code = new MagicString(context.code); - code.replace(/__GENERATE_JSX__\((\d+), ?(\d+)\)/g, (_, $1, $2) => { - return gen(parseInt($1, 10), parseInt($2, 10)); - }); + api.transform({}, (context) => { + const re = /__GENERATE_JSX__\((\d+), ?(\d+)\)/g; + if (!re.test(context.code)) return null; + const code = new MagicString(context.code); + code.replace(re, (_, $1, $2) => gen(parseInt($1, 10), parseInt($2, 10))); const sourceMap = code.generateMap({ hires: true, includeContent: true, source: context.resourcePath, }); return { code: code.toString(), map: sourceMap, }; });
📜 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 (9)
benchmark/react/cases/006-fuzz-complex-100/index.tsx(1 hunks)benchmark/react/cases/006-fuzz-complex-200/index.tsx(1 hunks)benchmark/react/lynx.config.ts(2 hunks)benchmark/react/package.json(3 hunks)benchmark/react/plugins/gen.ts(1 hunks)benchmark/react/plugins/pluginGenJSX.mjs(1 hunks)benchmark/react/rspeedy-env.d.ts(1 hunks)benchmark/react/scripts/tryGen.ts(1 hunks)benchmark/react/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- benchmark/react/rspeedy-env.d.ts
- benchmark/react/tsconfig.json
- benchmark/react/cases/006-fuzz-complex-100/index.tsx
- benchmark/react/package.json
- benchmark/react/lynx.config.ts
- benchmark/react/cases/006-fuzz-complex-200/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
benchmark/react/plugins/gen.ts
🧬 Code graph analysis (1)
benchmark/react/scripts/tryGen.ts (1)
benchmark/react/plugins/gen.ts (1)
gen(452-452)
🔇 Additional comments (1)
benchmark/react/plugins/gen.ts (1)
426-451: Confirm call-site contract: GENERATE_JSX replacement yields a factory that expects {useState}.The generator returns
(({useState}) => () => JSX); ensure call sites invoke it with the proper object (e.g.,__GENERATE_JSX__(s, c)({ useState })) or that downstream transformation injects the call.Would you like a quick repo scan script to verify usages in the benchmark cases?
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
benchmark/react/plugins/gen.ts (5)
241-242: Component names must start with uppercase for JSX recognition.Lowercase component names are treated as intrinsic HTML elements in JSX, not as React components. Line 241 generates a component name without ensuring it starts uppercase.
Apply this fix to ensure uppercase component names:
- const fcName = genJsIdentifier(r, ''); + const fcName = genJsIdentifier(r, 'C');
258-261: De-duplicate merged refVars to prevent duplicate parameter names.When merging
jsx1.refVarsandjsx2.refVars, duplicateidvalues can create duplicate parameter names in the generated component function, causing syntax errors.Apply this fix to de-duplicate refVars:
- const [refVars, refStates] = divideRandomly(r, [ - ...jsx1.refVars, - ...jsx2.refVars, - ], COMPONENT_PROPS_RATE); + const allRefVars: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of [...jsx1.refVars, ...jsx2.refVars]) { + if (!seen.has(v.id)) { seen.add(v.id); allRefVars.push(v); } + } + } + const [refVars, refStates] = divideRandomly(r, allRefVars, COMPONENT_PROPS_RATE);
288-293: Apply same de-duplication for single-subtree refVars.The same issue exists when generating components without children - duplicate
refVarids can still occur within a single subtree.Apply this fix to de-duplicate single subtree refVars:
- const jsx = genJSX(r, complexity, 0); - const [refVars, refStates] = divideRandomly( - r, - jsx.refVars, - COMPONENT_PROPS_RATE, - ); + const jsx = genJSX(r, complexity, 0); + const uniq: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of jsx.refVars) { + if (!seen.has(v.id)) { seen.add(v.id); uniq.push(v); } + } + } + const [refVars, refStates] = divideRandomly(r, uniq, COMPONENT_PROPS_RATE);
367-415: Add guaranteed progress fallback to prevent infinite loops.When
complexity === 1, several code paths don't decrement complexity, risking infinite loops. For example, the dynamic text branch at line 381 requirescomplexity > 2, and the static text branch at line 387 requirescomplexity > 1.Apply this fix to guarantee forward progress:
if (complexity > 0) { while (true) { + const before = complexity; // The more deeper the nesting, the more likely to generate a // ending (a Function Component without children or a text node) if (r.random() > 1 / 2 ** depth) { if (r.random() < COMPONENT_RATE) { const fc = genFC(r, complexity, undefined); code += fc.code; refVars.push(...fc.refVars); decls.push(...fc.decls); complexity -= fc.usedComplexity; usedComplexity += fc.usedComplexity; } else { if (r.random() < DYNAMIC_TEXT_RATE && complexity > 2) { const id = genJsIdentifier(r); code += `<text>{${id}}</text>`; refVars.push({ id, type: 'string' }); complexity -= 2; usedComplexity += 2; } else if (complexity > 1) { code += `<text>${genStr(r)}</text>`; complexity -= 1; usedComplexity += 1; } } } else { const jsx = genJSX(r, ~~(complexity / 2), depth + 1); complexity -= jsx.usedComplexity; usedComplexity += jsx.usedComplexity; if (r.random() < COMPONENT_RATE) { const fc = genFC(r, complexity, jsx.code); code += fc.code; refVars.push(...jsx.refVars, ...fc.refVars); decls.push(...jsx.decls, ...fc.decls); complexity -= fc.usedComplexity; usedComplexity += fc.usedComplexity; } else { code += jsx.code; refVars.push(...jsx.refVars); decls.push(...jsx.decls); } } + + // Force forward progress if nothing was consumed in this iteration. + if (complexity === before) { + code += `<text>${genStr(r)}</text>`; + complexity -= 1; + usedComplexity += 1; + } if (complexity <= 0) { break; } } }
449-461: De-duplicate declarations and refVars at top-level, and use proper value initialization.The generated code currently has two issues:
- Line 454 uses hardcoded string literal syntax instead of proper type-based value generation
- Duplicate declarations and refVars can cause "Identifier already declared" errors
Apply this fix to resolve both issues:
+ const declMap = new Map<string, string>(); + for (const d of g.decls) if (!declMap.has(d.id)) declMap.set(d.id, d.code); + const uniqRefVars: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of g.refVars) if (!seen.has(v.id)) { seen.add(v.id); uniqRefVars.push(v); } + } return `(({useState}, genRandom, genValue) => { -${g.decls.map(d => d.code).join('\n')} +${[...declMap.values()].join('\n')} return ({seed}) => { const r = { random: genRandom(seed) } ${ - g.refVars.map(v => `const ${v.id} = genValue(r, { type: "${v.type}" });`) + uniqRefVars.map(v => `const ${v.id} = genValue(r, { type: "${v.type}" });`) .join( '\n', ) } return (${g.code}) }; })`;
🧹 Nitpick comments (4)
benchmark/react/package.json (1)
6-26: Ensure builds precede runs to avoid stale/missing bundlesRecommend adding pre-scripts so local and CI runs don’t fail when dist/ is absent.
Apply:
"scripts": { - "bench": "pnpm run --sequential --stream --aggregate-output '/^bench:.*/'", + "prebench": "pnpm run build", + "bench": "pnpm run --sequential --stream --aggregate-output '/^bench:.*/'", @@ - "perfetto": "pnpm run --sequential --stream --aggregate-output '/^perfetto:.*/'", + "preperfetto": "pnpm run build", + "perfetto": "pnpm run --sequential --stream --aggregate-output '/^perfetto:.*/'",benchmark/react/cases/006-fuzz-complex-1000/index.tsx (1)
19-28: Make Codspeed stop resilientIf process() throws, Codspeed.stopBenchmark() won’t run. Wrap start/stop in try/finally.
useEffect(() => { setSeed(101); setStopBenchmark(true); - Codspeed.startBenchmark(); - process(); - Codspeed.stopBenchmark(); - Codspeed.setExecutedBenchmark( - `${__REPO_FILEPATH__}::${__webpack_chunkname__}-process`, - ); + try { + Codspeed.startBenchmark(); + process(); + } finally { + Codspeed.stopBenchmark(); + Codspeed.setExecutedBenchmark( + `${__REPO_FILEPATH__}::${__webpack_chunkname__}-process`, + ); + } }, []);benchmark/react/cases/006-fuzz-complex-2000/index.tsx (2)
19-28: Apply the same try/finally around CodspeedMirror the resilience tweak here as well.
useEffect(() => { setSeed(101); setStopBenchmark(true); - Codspeed.startBenchmark(); - process(); - Codspeed.stopBenchmark(); - Codspeed.setExecutedBenchmark( - `${__REPO_FILEPATH__}::${__webpack_chunkname__}-process`, - ); + try { + Codspeed.startBenchmark(); + process(); + } finally { + Codspeed.stopBenchmark(); + Codspeed.setExecutedBenchmark( + `${__REPO_FILEPATH__}::${__webpack_chunkname__}-process`, + ); + } }, []);
16-35: Reduce duplication (optional)If chunking/naming doesn’t depend on file boundaries, you could factor a tiny helper to create the App given a complexity, and import it from both cases to keep them in lockstep.
📜 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 (9)
benchmark/react/cases/006-fuzz-complex-1000/index.tsx(1 hunks)benchmark/react/cases/006-fuzz-complex-2000/index.tsx(1 hunks)benchmark/react/lynx.config.ts(2 hunks)benchmark/react/package.json(3 hunks)benchmark/react/plugins/gen.ts(1 hunks)benchmark/react/plugins/pluginGenJSX.mjs(1 hunks)benchmark/react/rspeedy-env.d.ts(1 hunks)benchmark/react/scripts/tryGen.ts(1 hunks)benchmark/react/tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- benchmark/react/tsconfig.json
- benchmark/react/scripts/tryGen.ts
- benchmark/react/rspeedy-env.d.ts
- benchmark/react/plugins/pluginGenJSX.mjs
🧰 Additional context used
🧠 Learnings (2)
📚 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:
benchmark/react/lynx.config.tsbenchmark/react/package.json
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
benchmark/react/plugins/gen.ts
🧬 Code graph analysis (2)
benchmark/react/lynx.config.ts (1)
benchmark/react/plugins/pluginGenJSX.mjs (2)
pluginGenJSX(12-35)pluginGenJSX(12-35)
benchmark/react/cases/006-fuzz-complex-2000/index.tsx (1)
benchmark/react/plugins/gen.ts (2)
genRandom(464-464)genValue(464-464)
🔇 Additional comments (12)
benchmark/react/package.json (2)
13-14: Bench scripts wired correctly to new fuzz targetsNames match entries in lynx.config.ts; wait-for-id aligns with the harness. LGTM.
23-24: Perfetto traces: unique outputs and ids look goodBoth commands emit distinct .ptrace files and wait for the same sentinel id; consistent with the harness. LGTM.
benchmark/react/lynx.config.ts (3)
49-56: Entries for fuzz: names and paths align with scriptsTargets exist and reference patchProfile + case index. LGTM.
9-9: Import verified — pluginGenJSX is ESM and imports MagicStringbenchmark/react/plugins/pluginGenJSX.mjs imports MagicString (line 5) and exports pluginGenJSX via ESM (line 12); the import in benchmark/react/lynx.config.ts is correct.
60-60: Verify GENERATE_JSX call-sites and harden transformPlacing pluginGenJSX() first is fine. The regex replacement can match inside strings/comments and only covers numeric literal args. The supplied rg returned no matches — cannot confirm call-sites. Action: confirm all GENERATE_JSX usages are numeric-literal-only and not in strings/comments, or replace the regex with an AST-based transform (recommended). If you keep regex, use a stricter pattern (e.g., (?<![A-Za-z0-9_$])GENERATE_JSX(\s*\d+\s*,\s*\d+\s*)).
Location: benchmark/react/lynx.config.ts:60
benchmark/react/cases/006-fuzz-complex-1000/index.tsx (2)
14-14: Macro call shape matches the transformNumeric literals ensure plugin replacement. LGTM.
8-10: Confirm preact->module exportsprocess
- I couldn't find any explicit export of
processin the repo, but there are multiple aliasing paths that mappreactimports to internal modules (so the import may be satisfied at build time):
- plugin-resolver sets aliases for many
preactentries (packages/rspeedy/plugin-react-alias/src/index.ts) and tests assertpreact$→ preact dist paths.- vitest config builds a
preactAliasfrom preact's package.json exports (packages/react/testing-library/src/vitest.config.js).- some configs map
preact→@lynx-js/react/internal(packages/react/refresh/rslib.config.ts), which means at least some environments resolvepreactto an internal module.- Because I could not find a definitive
export { process }orexport function processin the repo source, this is inconclusive: the import may rely on an external preact package (or a compiled runtime) that actually providesprocess.Action: Manually verify that the resolver used in the target environment maps 'preact' to a module that exports
process(either the external preact package you depend on or@lynx-js/react/internal). If you want I can run targeted checks against the resolved package.json exports or the installed preact package, but I need the environment paths or to run more scripts.benchmark/react/cases/006-fuzz-complex-2000/index.tsx (2)
14-14: Macro call shape matches the transformSame as 1000 case; replacement should be deterministic for complexity 2000. LGTM.
8-10: Verify the 'process' import alias in the 2000 fuzz case
Same alias concern as the 1000 case — ensure the 'process' import from 'preact' resolves correctly in benchmark/react/cases/006-fuzz-complex-2000/index.tsx (also present in 006-fuzz-complex-1000).benchmark/react/plugins/gen.ts (3)
23-33: LGTM! Clean type-based value generation.The
genValuefunction correctly handles different RefVar types with appropriate literal syntax. The error throwing for unreachable cases follows the established pattern in the codebase.
428-440: LGTM! Robust deterministic RNG implementation.The Robert Jenkins 32-bit hash function provides good distribution properties for seeded random generation, ensuring reproducible results for benchmark consistency.
101-233: Well-structured attribute generation with comprehensive coverage.The
genAttrfunction covers all necessary attribute types (standard attrs, datasets, events, classes, IDs) with appropriate dynamic/static variants. The complexity tracking and key deduplication logic is sound.
| runAfterLoadScript(() => { | ||
| root.render( | ||
| <App />, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Globals typing coverage
Ensure rspeedy-env.d.ts declares runAfterLoadScript, Codspeed, REPO_FILEPATH, and webpack_chunkname so TS stays clean without local ignores.
🏁 Script executed:
#!/bin/bash
# Check ambient declarations for globals/macros
rg -nP -C1 'declare\s+(var|const|function)\s+(runAfterLoadScript|Codspeed|__REPO_FILEPATH__|__webpack_chunkname__)' benchmark/react/rspeedy-env.d.tsLength of output: 215
Add ambient declaration for webpack_chunkname in benchmark/react/rspeedy-env.d.ts
rspeedy-env.d.ts already declares Codspeed, runAfterLoadScript, and REPO_FILEPATH; add the missing ambient declaration:
declare const webpack_chunkname: string;
🤖 Prompt for AI Agents
In benchmark/react/cases/006-fuzz-complex-1000/index.tsx around lines 37 to 41,
the environment declaration file rspeedy-env.d.ts is missing an ambient
declaration for __webpack_chunkname__, causing TypeScript to error when that
global is used; add the line declare const __webpack_chunkname__: string; to
rspeedy-env.d.ts alongside the existing declarations (Codspeed,
runAfterLoadScript, __REPO_FILEPATH__) so the global is recognized by the
compiler.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
benchmark/react/plugins/gen.ts (7)
35-43: Sanitize generated identifiers to avoid invalid JS (hyphens, leading digits).human-id may emit hyphens or digit-starting tokens; current output can produce invalid identifiers used in vars/JSX tags.
Apply:
-const genJsIdentifier = (r: Random, prefix = 'value') => { +const genJsIdentifier = (r: Random, prefix = 'value') => { const oldRandom = Math.random; try { Math.random = () => r.random(); - return `${prefix}${humanId({ adjectiveCount: 1 })}`; + const raw = humanId({ adjectiveCount: 1 }); + return sanitizeIdentifier(`${prefix}${raw}`); } finally { Math.random = oldRandom; } };Add helper (see next comment) before this function.
34-35: Add sanitizeIdentifier helper.+ +// Ensures a valid JS identifier (no hyphens, no leading digits). +const sanitizeIdentifier = (s: string): string => { + let id = s.replace(/[^A-Za-z0-9_$]/g, '_'); + if (/^[0-9]/.test(id)) id = `v_${id}`; + return id; +};
241-243: Ensure component names start with uppercase so JSX treats them as components.- const fcName = genJsIdentifier(r, ''); + const fcName = genJsIdentifier(r, 'C');
255-263: De-duplicate merged refVars before splitting; avoids duplicate prop/state ids.- const [refVars, refStates] = divideRandomly(r, [ - ...jsx1.refVars, - ...jsx2.refVars, - ], COMPONENT_PROPS_RATE); + const allRefVars: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of [...jsx1.refVars, ...jsx2.refVars]) { + if (!seen.has(v.id)) { seen.add(v.id); allRefVars.push(v); } + } + } + const [refVars, refStates] = divideRandomly(r, allRefVars, COMPONENT_PROPS_RATE);
288-294: Also de-duplicate refVars when there’s no children.- const [refVars, refStates] = divideRandomly( - r, - jsx.refVars, - COMPONENT_PROPS_RATE, - ); + const uniq: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of jsx.refVars) { + if (!seen.has(v.id)) { seen.add(v.id); uniq.push(v); } + } + } + const [refVars, refStates] = divideRandomly(r, uniq, COMPONENT_PROPS_RATE);
367-415: Possible non-terminating loop when complexity is 1; guarantee forward progress.if (complexity > 0) { while (true) { + const before = complexity; // The more deeper the nesting, the more likely to generate a // ending (a Function Component without children or a text node) if (r.random() > 1 / 2 ** depth) { // generate a ending @@ } else { // generate a nesting - const jsx = genJSX(r, ~~(complexity * r.random()), depth + 1); + const jsx = genJSX(r, ~~(complexity * r.random()), depth + 1); complexity -= jsx.usedComplexity; usedComplexity += jsx.usedComplexity; if (r.random() < COMPONENT_RATE) { const fc = genFC(r, complexity, jsx.code); code += fc.code; refVars.push(...fc.refVars, ...jsx.refVars); decls.push(...fc.decls, ...jsx.decls); complexity -= fc.usedComplexity; usedComplexity += fc.usedComplexity; } else { code += jsx.code; refVars.push(...jsx.refVars); decls.push(...jsx.decls); } } + // Force progress if nothing consumed in this iteration. + if (complexity === before) { + code += `<text>${genStr(r)}</text>`; + complexity -= 1; + usedComplexity += 1; + } + if (complexity <= 0) { break; } } }
449-461: De-duplicate decls and refVars before emitting to avoid “Identifier has already been declared”.- return `(({useState}, genRandom, genValue) => { -${g.decls.map(d => d.code).join('\n')} -return ({seed}) => { - const r = { random: genRandom(seed) } - ${ - g.refVars.map(v => `const ${v.id} = genValue(r, { type: "${v.type}" });`) - .join( - '\n', - ) - } - return (${g.code}) -}; -})`; + const declMap = new Map<string, string>(); + for (const d of g.decls) if (!declMap.has(d.id)) declMap.set(d.id, d.code); + const uniqRefVars: RefVar[] = []; + { + const seen = new Set<string>(); + for (const v of g.refVars) if (!seen.has(v.id)) { seen.add(v.id); uniqRefVars.push(v); } + } + return `(({useState}, genRandom, genValue) => { +${[...declMap.values()].join('\n')} +return ({seed}) => { + const r = { random: genRandom(seed) } + ${ + uniqRefVars.map(v => `const ${v.id} = genValue(r, { type: "${v.type}" });`).join('\n') + } + return (${g.code}) +}; +})`;
🧹 Nitpick comments (1)
benchmark/react/plugins/gen.ts (1)
205-206: Nit: remove dead expression in MT_Event handler.The string literal inside the handler is a no-op. Return or comment it for clarity.
- code: `${key}={() => { 'main-thread' }}`, + code: `${key}={() => { /* main-thread */ }}`,
📜 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 (9)
benchmark/react/cases/006-fuzz-complex-1000/index.tsx(1 hunks)benchmark/react/cases/006-fuzz-complex-2000/index.tsx(1 hunks)benchmark/react/lynx.config.ts(2 hunks)benchmark/react/package.json(3 hunks)benchmark/react/plugins/gen.ts(1 hunks)benchmark/react/plugins/pluginGenJSX.mjs(1 hunks)benchmark/react/rspeedy-env.d.ts(1 hunks)benchmark/react/scripts/tryGen.ts(1 hunks)benchmark/react/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- benchmark/react/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (6)
- benchmark/react/cases/006-fuzz-complex-1000/index.tsx
- benchmark/react/scripts/tryGen.ts
- benchmark/react/plugins/pluginGenJSX.mjs
- benchmark/react/rspeedy-env.d.ts
- benchmark/react/package.json
- benchmark/react/lynx.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
benchmark/react/plugins/gen.ts
🧬 Code graph analysis (1)
benchmark/react/cases/006-fuzz-complex-2000/index.tsx (2)
benchmark/react/plugins/gen.ts (2)
genRandom(464-464)genValue(464-464)packages/react/transform/src/wasm.js (1)
view(24-28)
⏰ 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)
There was a problem hiding this comment.
Pull Request Overview
This PR enables fuzz benchmark testing by adding a deterministic JSX generator for creating complex, repeatable UI components. The implementation supports generating nested React components with various attributes, events, and dynamic content at specified complexity levels (1000 and 2000 components).
- Adds a plugin system for JSX generation during build time using
__GENERATE_JSX__placeholders - Implements two new benchmark cases with high complexity (1000 and 2000 components)
- Provides tooling for previewing generated JSX and running benchmark/profiling tests
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| benchmark/react/tsconfig.json | Updates TypeScript config to include new plugin and script directories |
| benchmark/react/scripts/tryGen.ts | Utility script for previewing generated JSX output from stdin |
| benchmark/react/plugins/pluginGenJSX.mjs | Build plugin that replaces __GENERATE_JSX__ calls with generated code |
| benchmark/react/plugins/gen.ts | Core JSX generator with deterministic random seeding and complexity control |
| benchmark/react/package.json | Adds benchmark scripts and human-id dependency for identifier generation |
| benchmark/react/lynx.config.ts | Enables the JSX generator plugin and adds new benchmark entry points |
| benchmark/react/cases/006-fuzz-complex-2000/index.tsx | Benchmark case for 2000-complexity JSX generation |
| benchmark/react/cases/006-fuzz-complex-1000/index.tsx | Benchmark case for 1000-complexity JSX generation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
This pull request has been automatically marked as stale because it has not had recent activity. If this pull request is still relevant, please leave any comment (for example, "bump"). |
Summary by CodeRabbit
New Features
Chores
Checklist