refactor: replace static component catalog JSON files with an automat…#2504
refactor: replace static component catalog JSON files with an automat…#2504PupilTong merged 5 commits intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: d56e4f4 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 |
📝 WalkthroughWalkthroughRemoved handwritten component catalog JSON schemas, added an AST-driven TypeScript catalog generator and build config, updated package exports to point to built artifacts, added Turborepo config, excluded the new tools dir from linters, and added an empty changeset file. 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 |
ede5127 to
927bb6a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
packages/genui/a2ui/tools/catalog_generator.ts (1)
75-84:typeChecker/checkerare created but never used.
program.getTypeChecker()is expensive and its result is passed through toparseInferredTypesonly to be ignored. Either remove the unused plumbing, or actually use the checker (e.g.,checker.getTypeAtLocation(bodyNode)+checker.typeToString) soparseInferredTypescan infer beyondstring/boolean/numberkeywords — which would also obviate much of the ad-hocinferTypeFromNodelogic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 75 - 84, The created TypeScript TypeChecker (checker from program.getTypeChecker() in generateSchema) is currently unused—either remove it or wire it into the type inference path: pass checker into parseInferredTypes and, inside parseInferredTypes (and where inferTypeFromNode is used), call checker.getTypeAtLocation(node) and checker.typeToString(...) to produce richer inferred types beyond simple string/number/boolean, then fall back to the existing inferTypeFromNode logic only when checker cannot resolve a type; alternatively, if you opt to keep parseInferredTypes as-is, remove program.getTypeChecker() and the checker variable to avoid expensive unused work.
🤖 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/package.json`:
- Around line 9-19: The package.json "types" fields are pointing to .ts files
that won't exist in dist; update the top-level "types" and the "./core" and
"./chat" export entries so their "types" values use the generated .d.ts files
(replace "./dist/index.ts", "./dist/core/index.ts", and "./dist/chat/index.ts"
with "./dist/index.d.ts", "./dist/core/index.d.ts", and "./dist/chat/index.d.ts"
respectively) to match the existing catalog .d.ts pattern and ensure TypeScript
consumers can find declarations.
- Around line 93-97: The package "build" script only runs
tools/catalog_generator.ts but does not compile TypeScript or emit the ./dist/**
files referenced by exports (e.g., ./dist/index.js, ./dist/core/index.js,
./dist/chat/index.js, ./dist/catalog/<Name>.js), causing consumers to get
ERR_MODULE_NOT_FOUND; update the build pipeline to first compile the project
(run tsc -p tsconfig.json or the repo-standard rslib build) to produce ./dist
JS/DTs, then run tools/catalog_generator.ts and ensure the generator writes into
./dist/catalog (fix the output-path in catalog_generator.ts if needed) so that
all export paths (dist/index, dist/core, dist/chat, dist/catalog/*) exist after
pnpm -F `@lynx-js/a2ui-reactlynx` build.
- Around line 107-109: package.json lists "zod" and "zod-to-json-schema" but
they are unused; either remove them or wire them into catalog generation: if
removing, delete the two entries from packages/genui/a2ui/package.json and run
yarn/npm to prune; if integrating, update catalog_generator.ts (the module that
currently imports typescript, node:fs, node:path, node:url and contains
inferTypeFromNode) to import zod and zod-to-json-schema, convert inferred
runtime prop shapes into Zod schemas (create Zod object schemas inside or
alongside inferTypeFromNode) and call zodToJsonSchema to emit JSON Schema for
downstream use, ensuring you reference the inferTypeFromNode flow so generated
schemas replace or augment the existing type inference fallback.
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 168-200: The generator currently can crash or silently leave dist/
partial because JSON.parse and filesystem ops inside the per-component loop are
unguarded; wrap the per-component processing block (the logic that reads
catalogJsonPath, parses JSON into fullJson, hydrates baseSchema using
accessedProps, creates outDir and writes catalog.json into DIST_CATALOG_DIR) in
a try/catch that logs the componentName and the caught error (use console.error
or the existing logger), sets process.exitCode = 1 and continues to the next
component instead of throwing; also add a top-level try/catch around the
generateSchema() invocation to catch unexpected errors, log them, and set
process.exitCode = 1 so CI fails when appropriate.
- Around line 95-161: The AST walker only handles ts.isFunctionDeclaration so
components defined as VariableDeclaration with ArrowFunction/FunctionExpression
(e.g., export const Button = (...) => ...) and parameters using destructuring
are skipped; update visitNode to also detect
VariableStatement/VariableDeclaration whose initializer is an ArrowFunction or
FunctionExpression whose name/text matches componentName and then run the same
body traversal (visitBody) on its .initializer.body; additionally, inside the
parameter handling logic extend the current destructuring branch to process
ParameterDeclaration nodes with ObjectBindingPattern (use the parameter's
elements to populate accessedProps with {type:'string'} like the existing
variable-declaration branch) and finally add a clear warning log when no
matching function/variable component was found after traversal so missing
inferred props are visible; reference visitNode, visitBody, propsName,
accessedProps, GENERIC_PROPS and parseInferredTypes when making these changes.
- Around line 167-197: The generator currently writes per-component JSON under
DIST_CATALOG_DIR/<componentName>/catalog.json which doesn't match package.json
exports expecting flat dist/catalog/<Name>.js/.d.ts; update the generator (the
code that builds baseSchema, finalSchema and writes files using catalogJsonPath,
outDir, DIST_CATALOG_DIR, componentName, finalSchema) to emit a flat JS module
and matching declaration instead of a nested JSON file: write
dist/catalog/<componentName>.js that exports the finalSchema as the default
export (e.g. "export default ...") and also write a minimal
dist/catalog/<componentName>.d.ts declaring the default export type, ensuring
fs.mkdirSync still creates dist/catalog and that existing JSON writing is
replaced or augmented so package exports resolve correctly.
- Around line 165-184: The merge currently always falls back to an empty
baseSchema because catalog.json files were removed (catalogJsonPath/baseSchema),
causing loss of authoritative enums, descriptions and complex schemas; either
restore the deleted source catalog.json files so the merge in
catalog_generator.ts (catalogJsonPath, baseSchema, accessedProps) picks them up,
or remove/replace the merge and harden the codegen path by enhancing
inferTypeFromNode to emit enums, object/array schemas (including
additionalProperties and required), oneOf unions, and preserve descriptions so
when hydrating accessedProps into baseSchema you retain full metadata instead of
string fallbacks.
---
Nitpick comments:
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 75-84: The created TypeScript TypeChecker (checker from
program.getTypeChecker() in generateSchema) is currently unused—either remove it
or wire it into the type inference path: pass checker into parseInferredTypes
and, inside parseInferredTypes (and where inferTypeFromNode is used), call
checker.getTypeAtLocation(node) and checker.typeToString(...) to produce richer
inferred types beyond simple string/number/boolean, then fall back to the
existing inferTypeFromNode logic only when checker cannot resolve a type;
alternatively, if you opt to keep parseInferredTypes as-is, remove
program.getTypeChecker() and the checker variable to avoid expensive unused
work.
🪄 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: a7c6e204-3a9a-4707-98ef-22eaf1123b40
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.changeset/seven-llamas-stare.mdbiome.jsonceslint.config.jspackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/Button/catalog.jsonpackages/genui/a2ui/src/catalog/Card/catalog.jsonpackages/genui/a2ui/src/catalog/CheckBox/catalog.jsonpackages/genui/a2ui/src/catalog/Column/catalog.jsonpackages/genui/a2ui/src/catalog/Divider/catalog.jsonpackages/genui/a2ui/src/catalog/Image/catalog.jsonpackages/genui/a2ui/src/catalog/List/catalog.jsonpackages/genui/a2ui/src/catalog/RadioGroup/catalog.jsonpackages/genui/a2ui/src/catalog/Row/catalog.jsonpackages/genui/a2ui/src/catalog/Text/catalog.jsonpackages/genui/a2ui/tools/catalog_generator.tspackages/genui/a2ui/turbo.json
💤 Files with no reviewable changes (10)
- packages/genui/a2ui/src/catalog/Card/catalog.json
- packages/genui/a2ui/src/catalog/List/catalog.json
- packages/genui/a2ui/src/catalog/Image/catalog.json
- packages/genui/a2ui/src/catalog/Text/catalog.json
- packages/genui/a2ui/src/catalog/Divider/catalog.json
- packages/genui/a2ui/src/catalog/RadioGroup/catalog.json
- packages/genui/a2ui/src/catalog/CheckBox/catalog.json
- packages/genui/a2ui/src/catalog/Row/catalog.json
- packages/genui/a2ui/src/catalog/Column/catalog.json
- packages/genui/a2ui/src/catalog/Button/catalog.json
Merging this PR will not alter performance
Comparing Footnotes
|
React Example#7558 Bundle Size — 225.23KiB (0%).d56e4f4(current) vs 13655ac main#7556(baseline) Bundle metrics
|
| Current #7558 |
Baseline #7556 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.57% |
44.57% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7558 |
Baseline #7556 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
79.47KiB |
79.47KiB |
Bundle analysis report Branch PupilTong:p/hw/auto-gen-catalog Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#690 Bundle Size — 196.39KiB (0%).d56e4f4(current) vs 13655ac main#688(baseline) Bundle metrics
|
| Current #690 |
Baseline #688 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
44.07% |
44.07% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #690 |
Baseline #688 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
85.15KiB |
85.15KiB |
Bundle analysis report Branch PupilTong:p/hw/auto-gen-catalog Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9130 Bundle Size — 900.11KiB (0%).d56e4f4(current) vs 13655ac main#9128(baseline) Bundle metrics
Bundle size by type
|
| Current #9130 |
Baseline #9128 |
|
|---|---|---|
495.97KiB |
495.97KiB |
|
401.92KiB |
401.92KiB |
|
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:p/hw/auto-gen-catalog Project dashboard
Generated by RelativeCI Documentation Report issue
React External#676 Bundle Size — 679.93KiB (0%).d56e4f4(current) vs 13655ac main#674(baseline) Bundle metrics
|
| Current #676 |
Baseline #674 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch PupilTong:p/hw/auto-gen-catalog Project dashboard
Generated by RelativeCI Documentation Report issue
|
Could we extract this tool into a separate package? |
|
It would be even better if tests. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/genui/a2ui/tools/catalog_generator.ts (2)
222-230:⚠️ Potential issue | 🔴 CriticalAlign generated output with the package export contract.
The generator writes
dist/catalog/<Component>/catalog.json, butpackages/genui/a2ui/package.jsonexports resolve to flat files likedist/catalog/Button.jsanddist/catalog/Button.d.ts. Consumers importing./catalog/Buttonwill not find the generated artifact.One possible direction
- const outDir = path.join(DIST_CATALOG_DIR, componentName); + const outDir = DIST_CATALOG_DIR; fs.mkdirSync(outDir, { recursive: true }); const finalSchema = { [componentName]: baseSchema }; const finalSchemaStr = JSON.stringify(finalSchema, null, 2); fs.writeFileSync( - path.join(outDir, 'catalog.json'), - finalSchemaStr + '\n', + path.join(outDir, `${componentName}.js`), + `export default ${finalSchemaStr};\n`, + ); + fs.writeFileSync( + path.join(outDir, `${componentName}.d.ts`), + 'declare const schema: Record<string, unknown>;\nexport default schema;\n', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 222 - 230, The generator currently writes a nested JSON file at outDir/catalog.json (using DIST_CATALOG_DIR, componentName, fs.writeFileSync and finalSchema) but package exports expect flat module files like dist/catalog/<Component>.js and .d.ts; update the write step to emit a JS module file named path.join(DIST_CATALOG_DIR, `${componentName}.js`) that exports the schema (e.g. module.exports = <finalSchema> or export default <finalSchema> consistent with your bundler) and also emit a corresponding TypeScript declaration path.join(DIST_CATALOG_DIR, `${componentName}.d.ts`) that declares the default export type, ensuring imports like './catalog/Button' resolve correctly instead of the current nested catalog.json file.
201-220:⚠️ Potential issue | 🟠 MajorDo not fall back to empty schemas after removing source
catalog.jsonfiles.With the static
src/catalog/*/catalog.jsonfiles removed, Line 204 becomes the common path, so the generated schema keeps only inferred prop names and loses authoritative metadata like descriptions, enums, required fields, and nested object shapes. This is especially risky for props such asButton.action, which are not representable as{ type: 'string' }.Either restore the source schema bases or make the generator emit the full schema fidelity directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 201 - 220, The generator currently falls back to an empty baseSchema when src/catalog/*/catalog.json is missing (catalogJsonPath -> baseSchema), causing loss of authoritative metadata; update the merge logic in catalog_generator.ts (symbols: catalogJsonPath, baseSchema, accessedProps, componentName, componentDir) to preserve full schema fidelity by: 1) attempting to load an alternative authoritative source for the component schema before defaulting to an empty object, 2) performing a deep merge of loaded baseSchema with inferred propSchema from accessedProps so descriptions, enums, required flags and nested object shapes are merged (not overwritten) into baseSchema.properties[propName], and 3) if no authoritative source exists, synthesize richer schema metadata from AST/component metadata rather than using a bare {type:'object'}/empty properties; ensure merge preserves existing complex entries like Button.action.
🤖 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/tools/catalog_generator.ts`:
- Around line 75-83: The generator currently logs when
getComponentIndexFiles(CATALOG_DIR) returns zero but continues; update
generateSchema to treat zero discovered entries as a hard failure by checking
indexFiles.length === 0 and aborting the process (throw an Error or call
process.exit(1)) with a descriptive message that includes CATALOG_DIR and that
no component entrypoints were found; ensure this check sits immediately after
obtaining indexFiles so the ts.createProgram call is not executed with an empty
file list.
---
Duplicate comments:
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 222-230: The generator currently writes a nested JSON file at
outDir/catalog.json (using DIST_CATALOG_DIR, componentName, fs.writeFileSync and
finalSchema) but package exports expect flat module files like
dist/catalog/<Component>.js and .d.ts; update the write step to emit a JS module
file named path.join(DIST_CATALOG_DIR, `${componentName}.js`) that exports the
schema (e.g. module.exports = <finalSchema> or export default <finalSchema>
consistent with your bundler) and also emit a corresponding TypeScript
declaration path.join(DIST_CATALOG_DIR, `${componentName}.d.ts`) that declares
the default export type, ensuring imports like './catalog/Button' resolve
correctly instead of the current nested catalog.json file.
- Around line 201-220: The generator currently falls back to an empty baseSchema
when src/catalog/*/catalog.json is missing (catalogJsonPath -> baseSchema),
causing loss of authoritative metadata; update the merge logic in
catalog_generator.ts (symbols: catalogJsonPath, baseSchema, accessedProps,
componentName, componentDir) to preserve full schema fidelity by: 1) attempting
to load an alternative authoritative source for the component schema before
defaulting to an empty object, 2) performing a deep merge of loaded baseSchema
with inferred propSchema from accessedProps so descriptions, enums, required
flags and nested object shapes are merged (not overwritten) into
baseSchema.properties[propName], and 3) if no authoritative source exists,
synthesize richer schema metadata from AST/component metadata rather than using
a bare {type:'object'}/empty properties; ensure merge preserves existing complex
entries like Button.action.
🪄 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: ad2ca232-9e01-475e-bb17-a106e7ab545b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/genui/a2ui/package.jsonpackages/genui/a2ui/tools/catalog_generator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/genui/a2ui/package.json
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/genui/a2ui/src/catalog/List/index.tsx (1)
42-76:⚠️ Potential issue | 🔴 CriticalFix the branch structure so lists render any children.
Static children are overwritten with
[], and dynamic template children never enter the rendering branch. As written,Listrenders an empty list for both supportedchildrenshapes.Proposed fix
let content: (ListItem | null)[] = []; if (Array.isArray(children)) { content = children.map((childId: string) => { const child = surface.components.get(childId); if (!child) return null; @@ component: childWithContext, }; }); - const componentId = template?.componentId ?? ''; + } else if (template) { + const componentId = template.componentId; const items = Array.isArray(listData) ? listData : []; content = items.map((item, index) => { @@ component: childWithContext, }; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/List/index.tsx` around lines 42 - 76, The branch logic currently maps Array.isArray(children) into content and then always overwrites content with items.map, so static children are discarded and template-driven lists never run correctly; change the control flow so you handle the two shapes separately: when Array.isArray(children) is true, build content from children (use surface.components.get(childId) and propagate dataContextPath as currently done) and return/assign that result, otherwise if template?.componentId and Array.isArray(listData) build content from listData using componentId, fullPath, and dataContextPath (preserve the key logic and childWithContext construction), ensuring you do not always reassign content after handling children.packages/genui/a2ui/src/catalog/Text/index.tsx (1)
17-22:⚠️ Potential issue | 🟠 MajorResolve
{ path }text bindings before rendering.Line 22 can receive the object branch from
TextProps.text;as stringonly silences TypeScript and does not resolve the binding at runtime.Proposed fix
import type { GenericComponentProps } from '../../core/types.js'; +import { useDataBinding } from '../../core/useDataBinding.js'; import './style.css'; @@ - const id = props.id; - const text = props.text; - const variant = props.variant as string | undefined ?? 'body'; + const { id, surface, dataContextPath } = props; + const textProp = props.text; + const [text] = useDataBinding<string>( + textProp && typeof textProp === 'object' ? textProp : undefined, + surface, + dataContextPath, + typeof textProp === 'string' ? textProp : '', + ); + const variant = props.variant ?? 'body'; @@ - {text as string} + {text}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Text/index.tsx` around lines 17 - 22, The component currently casts props.text to a string and renders it directly (see the text variable and the JSX <text key={id} ...>{text as string}</text>), but TextProps.text can be an object binding like { path } that must be resolved at runtime; change the rendering to first resolve the binding to a plain string (e.g., implement or call a resolver like resolveTextBinding(text) or resolveBinding(text) that handles object branches and returns a string or fallback) and then render the resolved string in place of the current cast, keeping the existing variant logic and key/id usage.packages/genui/a2ui/src/catalog/Image/index.tsx (1)
26-42:⚠️ Potential issue | 🟠 MajorResolve bound URLs and honor the exposed image props.
Line 41 can pass a
{ path }object assrc, and Line 42 ignoresvariantby hard-codingmediumFeature. Either wirefitto the supported styling/attribute path as well, or remove it fromImageProps.Proposed fix
import { useEffect, useState } from '@lynx-js/react'; import type { GenericComponentProps } from '../../core/types.js'; +import { useDataBinding } from '../../core/useDataBinding.js'; @@ - const { id, url } = props; + const { id, url, variant = 'mediumFeature', surface, dataContextPath } = props; + const [resolvedUrl] = useDataBinding<string>( + url && typeof url === 'object' ? url : undefined, + surface, + dataContextPath, + typeof url === 'string' ? url : '', + ); @@ useEffect(() => { setHasError(false); - }, [url]); + }, [resolvedUrl]); const finalSrc = hasError ? 'https://lf3-static.bytednsdoc.com/obj/eden-cn/zalzzh-ukj-lapzild-shpjpmmv-eufs/ljhwZthlaukjlkulzlp/built-in-images/logo.png' - : url; + : resolvedUrl; @@ - src={finalSrc as string} - className='a2ui-image mediumFeature' + src={finalSrc} + className={`a2ui-image ${variant}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Image/index.tsx` around lines 26 - 42, The Image component currently assumes src is a string and hard-codes the CSS variant; update it to accept either a string or an object with a path (handle when props.url is { path }) by resolving finalSrc = typeof url === 'string' ? url : url?.path, keep the hasError fallback as now, and replace the hard-coded 'mediumFeature' class with the incoming props.variant (or remove variant from ImageProps if not used). Also wire the props.fit through to the rendered element (e.g., map fit to a class or style) so the component honors ImageProps.fit instead of ignoring it.packages/genui/a2ui/src/catalog/CheckBox/index.tsx (1)
16-27:⚠️ Potential issue | 🟠 MajorResolve bound
labelandvaluebefore using them as primitives.With
value: { path: string }, Line 25 always renders checked and Line 19 always writesfalse;label as stringhas the same unresolved-object problem.Proposed fix
import type { GenericComponentProps } from '../../core/types.js'; +import { useDataBinding } from '../../core/useDataBinding.js'; @@ - const { id, label = 'CheckBox', value, setValue } = props; + const { + id, + label: labelProp = 'CheckBox', + value: valueProp, + setValue, + surface, + dataContextPath, + } = props; + + const [label] = useDataBinding<string>( + labelProp && typeof labelProp === 'object' ? labelProp : undefined, + surface, + dataContextPath, + typeof labelProp === 'string' ? labelProp : 'CheckBox', + ); + const [value] = useDataBinding<boolean>( + valueProp && typeof valueProp === 'object' ? valueProp : undefined, + surface, + dataContextPath, + typeof valueProp === 'boolean' ? valueProp : false, + ); @@ - <text className='checkbox-label'>{label as string}</text> + <text className='checkbox-label'>{label}</text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/CheckBox/index.tsx` around lines 16 - 27, The CheckBox component is using label and value directly though they may be bound objects (e.g., {path: string}); update the component to normalize/unpack bound values before use: inside the CheckBox (props destructuring and before render) detect if value is an object and extract a boolean (e.g., value = typeof value === 'object' ? resolveBound(value) : value) and similarly resolve label to a string, then use those primitives in the JSX and in handleChange; also ensure handleChange calls setValue with the correct shape expected by the binding (pass the resolved primitive or wrap it back into the binding shape) so toggling and rendering use consistent types (refer to symbols: CheckBox component, props.id, props.label, props.value, setValue, and handleChange).
♻️ Duplicate comments (3)
packages/genui/a2ui/tools/catalog_generator.ts (3)
175-184:⚠️ Potential issue | 🟠 MajorFail the generator when discovery or schema resolution produces no output.
Line 177 can report
0component files and still exit successfully; Line 274 also only warns when a component schema cannot be resolved. That can ship a build with missing catalog artifacts.Proposed fix
function generateSchema() { const indexFiles = getComponentIndexFiles(CATALOG_DIR); + if (indexFiles.length === 0) { + throw new Error(`No component index.tsx files found under ${CATALOG_DIR}`); + } console.log(`Found ${indexFiles.length} component files`); @@ if (baseSchema) { @@ console.log(`Generated strict schema for ${componentName}`); } else { - console.warn(`[Warning] Could not resolve schema for ${componentName}`); + throw new Error(`Could not resolve schema for ${componentName}`); } } }Also applies to: 273-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 175 - 184, The generator currently continues on zero discovery or unresolved schemas; update generateSchema so it fails fast and returns a non-zero exit when no components are found or when any component schema cannot be resolved: after calling getComponentIndexFiles(CATALOG_DIR) check indexFiles.length and throw an Error (or call process.exit(1)) with a descriptive message if 0; likewise, where component schemas are resolved (the schema resolution loop / function used around the later resolution step that currently only console.warn's), detect any unresolved schema(s) and throw an Error (or process.exit(1)) listing the affected component identifiers so the build fails instead of emitting incomplete catalog artifacts. Ensure you change both the initial discovery check and the schema-resolution warning path in generateSchema.
196-206:⚠️ Potential issue | 🟠 MajorSupport non-
functioncomponent declarations or fail clearly.The AST matcher still only accepts
FunctionDeclarationnamed exactly like the component directory.export const Button = (...) => ..., default exports, or wrapper-assigned components will be skipped and hit the warning path instead of generating a catalog.#!/bin/bash # Verify catalog component declaration forms. # Expected: every src/catalog/*/index.tsx component either uses `export function <DirName>` # or the generator handles the declaration form used here. rg -nP --type=tsx -C4 'export\s+(function|const|default)|=\s*(\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>' packages/genui/a2ui/src/catalog🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 196 - 206, The current visitNode only handles ts.isFunctionDeclaration named componentName and skips arrow/const/default/wrapper forms; update visitNode to also detect ts.VariableStatement / ts.VariableDeclaration where the name equals componentName and the initializer is a ts.ArrowFunction or ts.FunctionExpression (and handle exported consts), detect ts.ExportAssignment/default exports that reference an identifier equal to componentName, and handle assigned wrappers (e.g., ts.CallExpression like forwardRef(...) or HOC calls) by resolving the referenced identifier to its declaration via checker.getSymbolAtLocation and then obtaining the type with checker.getTypeAtLocation; pass that resolved type into processComponentPropsType to set baseSchema. Also, when an unsupported form is encountered, replace the silent warning with a clear error message that includes componentName and the node kind so the failure is explicit. Ensure references to visitNode, componentName, checker, processComponentPropsType, and baseSchema are used to locate and modify the code.
260-270:⚠️ Potential issue | 🔴 CriticalVerify the generated path matches the exported catalog subpaths.
This still writes nested JSON at
dist/catalog/<Name>/catalog.json. Ifpackage.jsonexports resolve catalog entries as built JS/DTS files, these artifacts will not satisfy those exports after build.#!/bin/bash # Verify package exports against generator output shape. # Expected: exports for catalog entries point to `dist/catalog/<Name>/catalog.json`, # or the generator emits the exact JS/DTS artifacts referenced by package.json. node -e " const fs = require('node:fs'); const pkg = JSON.parse(fs.readFileSync('packages/genui/a2ui/package.json', 'utf8')); console.log(JSON.stringify(pkg.exports, null, 2)); "🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 260 - 270, The generator currently writes JSON to DIST_CATALOG_DIR/componentName/catalog.json which may not match package.json exports; update the logic in catalog_generator.ts (variables/functions: DIST_CATALOG_DIR, componentName, finalSchemaStr, outDir) to either (A) compute outDir/path from the package.json.exports entry for the catalog subpath and write catalog.json to that exact exported path, or (B) emit the JS/DTS artifacts referenced by package.json.exports (rather than only a raw JSON) so the built artifacts match the package exports; adjust writeFileSync target accordingly and add a validation step that loads package.json.exports to assert the generated file path equals the exported path.
🤖 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/src/catalog/RadioGroup/index.tsx`:
- Around line 19-33: The props type advertises path-backed values but RadioGroup
doesn't resolve them; update the contract to match implementation by changing
RadioGroupComponentProps so items: string[] and value: string (remove the | {
path: string } unions), and update any usage of RadioGroup to pass
already-resolved values; alternatively, if you must keep path support, implement
internal resolution by calling the same resolver used elsewhere (e.g.,
A2UIRender.useResolvedProps or useDataBinding) inside RadioGroup and use the
resolved value variable instead of raw props.value, ensuring
RadioGroupComponentProps stays consistent with the chosen approach (either
remove path unions or wire in resolution via RadioGroup/useDataBinding).
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 209-255: In processComponentPropsType, the generated schema lacks
an explicit object type so non-object values can match; set the root schema to
be an object by assigning schema.type = 'object' (on the schema object created
in processComponentPropsType) before returning, and while here tighten the
required handling by only attaching schema.required when required.length > 0
instead of >= 0 to avoid emitting an empty required array unnecessarily.
---
Outside diff comments:
In `@packages/genui/a2ui/src/catalog/CheckBox/index.tsx`:
- Around line 16-27: The CheckBox component is using label and value directly
though they may be bound objects (e.g., {path: string}); update the component to
normalize/unpack bound values before use: inside the CheckBox (props
destructuring and before render) detect if value is an object and extract a
boolean (e.g., value = typeof value === 'object' ? resolveBound(value) : value)
and similarly resolve label to a string, then use those primitives in the JSX
and in handleChange; also ensure handleChange calls setValue with the correct
shape expected by the binding (pass the resolved primitive or wrap it back into
the binding shape) so toggling and rendering use consistent types (refer to
symbols: CheckBox component, props.id, props.label, props.value, setValue, and
handleChange).
In `@packages/genui/a2ui/src/catalog/Image/index.tsx`:
- Around line 26-42: The Image component currently assumes src is a string and
hard-codes the CSS variant; update it to accept either a string or an object
with a path (handle when props.url is { path }) by resolving finalSrc = typeof
url === 'string' ? url : url?.path, keep the hasError fallback as now, and
replace the hard-coded 'mediumFeature' class with the incoming props.variant (or
remove variant from ImageProps if not used). Also wire the props.fit through to
the rendered element (e.g., map fit to a class or style) so the component honors
ImageProps.fit instead of ignoring it.
In `@packages/genui/a2ui/src/catalog/List/index.tsx`:
- Around line 42-76: The branch logic currently maps Array.isArray(children)
into content and then always overwrites content with items.map, so static
children are discarded and template-driven lists never run correctly; change the
control flow so you handle the two shapes separately: when
Array.isArray(children) is true, build content from children (use
surface.components.get(childId) and propagate dataContextPath as currently done)
and return/assign that result, otherwise if template?.componentId and
Array.isArray(listData) build content from listData using componentId, fullPath,
and dataContextPath (preserve the key logic and childWithContext construction),
ensuring you do not always reassign content after handling children.
In `@packages/genui/a2ui/src/catalog/Text/index.tsx`:
- Around line 17-22: The component currently casts props.text to a string and
renders it directly (see the text variable and the JSX <text key={id} ...>{text
as string}</text>), but TextProps.text can be an object binding like { path }
that must be resolved at runtime; change the rendering to first resolve the
binding to a plain string (e.g., implement or call a resolver like
resolveTextBinding(text) or resolveBinding(text) that handles object branches
and returns a string or fallback) and then render the resolved string in place
of the current cast, keeping the existing variant logic and key/id usage.
---
Duplicate comments:
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 175-184: The generator currently continues on zero discovery or
unresolved schemas; update generateSchema so it fails fast and returns a
non-zero exit when no components are found or when any component schema cannot
be resolved: after calling getComponentIndexFiles(CATALOG_DIR) check
indexFiles.length and throw an Error (or call process.exit(1)) with a
descriptive message if 0; likewise, where component schemas are resolved (the
schema resolution loop / function used around the later resolution step that
currently only console.warn's), detect any unresolved schema(s) and throw an
Error (or process.exit(1)) listing the affected component identifiers so the
build fails instead of emitting incomplete catalog artifacts. Ensure you change
both the initial discovery check and the schema-resolution warning path in
generateSchema.
- Around line 196-206: The current visitNode only handles
ts.isFunctionDeclaration named componentName and skips
arrow/const/default/wrapper forms; update visitNode to also detect
ts.VariableStatement / ts.VariableDeclaration where the name equals
componentName and the initializer is a ts.ArrowFunction or ts.FunctionExpression
(and handle exported consts), detect ts.ExportAssignment/default exports that
reference an identifier equal to componentName, and handle assigned wrappers
(e.g., ts.CallExpression like forwardRef(...) or HOC calls) by resolving the
referenced identifier to its declaration via checker.getSymbolAtLocation and
then obtaining the type with checker.getTypeAtLocation; pass that resolved type
into processComponentPropsType to set baseSchema. Also, when an unsupported form
is encountered, replace the silent warning with a clear error message that
includes componentName and the node kind so the failure is explicit. Ensure
references to visitNode, componentName, checker, processComponentPropsType, and
baseSchema are used to locate and modify the code.
- Around line 260-270: The generator currently writes JSON to
DIST_CATALOG_DIR/componentName/catalog.json which may not match package.json
exports; update the logic in catalog_generator.ts (variables/functions:
DIST_CATALOG_DIR, componentName, finalSchemaStr, outDir) to either (A) compute
outDir/path from the package.json.exports entry for the catalog subpath and
write catalog.json to that exact exported path, or (B) emit the JS/DTS artifacts
referenced by package.json.exports (rather than only a raw JSON) so the built
artifacts match the package exports; adjust writeFileSync target accordingly and
add a validation step that loads package.json.exports to assert the generated
file path equals the exported path.
🪄 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: 645d5bd6-96fe-42fd-bd59-d776255032b4
📒 Files selected for processing (11)
packages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Card/index.tsxpackages/genui/a2ui/src/catalog/CheckBox/index.tsxpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/catalog/Divider/index.tsxpackages/genui/a2ui/src/catalog/Image/index.tsxpackages/genui/a2ui/src/catalog/List/index.tsxpackages/genui/a2ui/src/catalog/RadioGroup/index.tsxpackages/genui/a2ui/src/catalog/Row/index.tsxpackages/genui/a2ui/src/catalog/Text/index.tsxpackages/genui/a2ui/tools/catalog_generator.ts
…ed TypeScript generator script
…date build process to use tsc for type definitions
cd56e71 to
f04b494
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/genui/a2ui/src/catalog/Row/index.tsx (1)
9-28: LGTM — optional nit on redundant casts.
RowPropsand the updatedRowsignature look good and match the generator's expectations (explicit exported props interface extendingGenericComponentProps). Minor:props.justifyandprops.alignare already the literal-union types, soas string | undefinedon lines 27–28 is unnecessary and widens the type for no reason.- const justify = props.justify as string | undefined ?? 'start'; - const align = props.align as string | undefined ?? 'stretch'; + const justify = props.justify ?? 'start'; + const align = props.align ?? 'stretch';Also worth noting:
children: string[] | { componentId: string; path: string }— the template-binding branch ({ componentId, path }) is accepted by the type but silently discarded byArray.isArray(children) ? children : []on line 29. If that branch isn't wired up yet, aTODOthere would help reviewers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Row/index.tsx` around lines 9 - 28, Remove the redundant casts on props.justify and props.align in the Row function (they are already the correct literal-union types via RowProps) and default them directly (e.g., use props.justify ?? 'start' and props.align ?? 'stretch'); also address the template-binding branch of children: if children can be { componentId, path } either handle that branch instead of silently discarding it where Array.isArray(children) ? children : [] is used, or add a clear TODO comment next to that conditional to indicate the template case is intentionally unimplemented.packages/genui/a2ui/src/catalog/Text/index.tsx (1)
7-18: LGTM — optional nit on redundant cast.The
TextPropsinterface and dot-notation access look clean. Minor:props.variantis already typed as the literal union |undefined, soas string | undefinedon line 18 (andas stringon line 22) is unnecessary and weakens type safety.Optional cleanup
- const text = props.text; - const variant = props.variant as string | undefined ?? 'body'; + const text = props.text; + const variant = props.variant ?? 'body'; ... - {text as string} + {typeof text === 'string' ? text : null}(The
textprop isstring | { path: string }, so rendering it directly asstringis actually unsafe when it's a path-binding object — worth handling explicitly.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/src/catalog/Text/index.tsx` around lines 7 - 18, Remove the redundant type casts on props.variant in the Text component: stop using "as string | undefined" and "as string" and instead rely on the declared TextProps union (variant?: 'h1'|'h2'|'h3'|'h4'|'h5'|'caption'|'body') and set a default like const variant = props.variant ?? 'body'; also make props.text handling explicit in the Text function (Text) by checking whether props.text is a string or an object ({ path }) before rendering or converting it, so you don't assume it's always a string.packages/genui/a2ui/tools/catalog_generator.ts (1)
209-256:processComponentPropsTypelargely duplicates the object branch ofparseType.Lines 209–256 re-implement what lines 117–164 of
parseTypealready do for object types; the only meaningful differences are (a) skipping properties whose declaring parent interface isGenericComponentProps/ComponentProps, and (b) not emittingtype: 'object'/additionalPropertiesat the root (both of which were flagged previously as missing).Consider collapsing to a single object-extraction helper that takes a predicate for property filtering, so both code paths stay in sync (e.g. fixes to JSDoc extraction, optionality handling, or index signatures only need to land once).
-function processComponentPropsType(type: ts.Type, checker: ts.TypeChecker) { - const schema: any = { properties: {} }; - ... -} +function processComponentPropsType(type: ts.Type, checker: ts.TypeChecker) { + return parseObjectType(type, checker, (p) => { + const decl = p.valueDeclaration || p.declarations?.[0]; + const parent = decl?.parent; + return !( + parent + && ts.isInterfaceDeclaration(parent) + && (parent.name.text === 'GenericComponentProps' + || parent.name.text === 'ComponentProps') + ); + }); +}Where
parseObjectTypeis the extracted shared helper used by both the union/object branch ofparseTypeand this function.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/genui/a2ui/tools/catalog_generator.ts` around lines 209 - 256, processComponentPropsType duplicates the object-handling logic in parseType; extract that shared logic into a new helper function (e.g. parseObjectType) that accepts a TypeScript object type, a checker, and an optional property-filter predicate, then have both parseType (object branch and object union branch) and processComponentPropsType call parseObjectType; ensure parseObjectType emits type: 'object', additionalProperties, index signatures, JSDoc descriptions, correct optionality detection (respecting SymbolFlags.Optional, questionToken and union-with-Undefined), and builds schema.required only when there are required props (use required.length > 0), and make processComponentPropsType pass a predicate that filters out properties whose declaration parent name is 'GenericComponentProps' or 'ComponentProps'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/genui/a2ui/src/catalog/Row/index.tsx`:
- Around line 9-28: Remove the redundant casts on props.justify and props.align
in the Row function (they are already the correct literal-union types via
RowProps) and default them directly (e.g., use props.justify ?? 'start' and
props.align ?? 'stretch'); also address the template-binding branch of children:
if children can be { componentId, path } either handle that branch instead of
silently discarding it where Array.isArray(children) ? children : [] is used, or
add a clear TODO comment next to that conditional to indicate the template case
is intentionally unimplemented.
In `@packages/genui/a2ui/src/catalog/Text/index.tsx`:
- Around line 7-18: Remove the redundant type casts on props.variant in the Text
component: stop using "as string | undefined" and "as string" and instead rely
on the declared TextProps union (variant?:
'h1'|'h2'|'h3'|'h4'|'h5'|'caption'|'body') and set a default like const variant
= props.variant ?? 'body'; also make props.text handling explicit in the Text
function (Text) by checking whether props.text is a string or an object ({ path
}) before rendering or converting it, so you don't assume it's always a string.
In `@packages/genui/a2ui/tools/catalog_generator.ts`:
- Around line 209-256: processComponentPropsType duplicates the object-handling
logic in parseType; extract that shared logic into a new helper function (e.g.
parseObjectType) that accepts a TypeScript object type, a checker, and an
optional property-filter predicate, then have both parseType (object branch and
object union branch) and processComponentPropsType call parseObjectType; ensure
parseObjectType emits type: 'object', additionalProperties, index signatures,
JSDoc descriptions, correct optionality detection (respecting
SymbolFlags.Optional, questionToken and union-with-Undefined), and builds
schema.required only when there are required props (use required.length > 0),
and make processComponentPropsType pass a predicate that filters out properties
whose declaration parent name is 'GenericComponentProps' or 'ComponentProps'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd43fb18-7cfb-48be-927e-b02a790ae736
📒 Files selected for processing (26)
.changeset/seven-llamas-stare.mdbiome.jsonceslint.config.jspackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/Button/catalog.jsonpackages/genui/a2ui/src/catalog/Button/index.tsxpackages/genui/a2ui/src/catalog/Card/catalog.jsonpackages/genui/a2ui/src/catalog/Card/index.tsxpackages/genui/a2ui/src/catalog/CheckBox/catalog.jsonpackages/genui/a2ui/src/catalog/CheckBox/index.tsxpackages/genui/a2ui/src/catalog/Column/catalog.jsonpackages/genui/a2ui/src/catalog/Column/index.tsxpackages/genui/a2ui/src/catalog/Divider/catalog.jsonpackages/genui/a2ui/src/catalog/Divider/index.tsxpackages/genui/a2ui/src/catalog/Image/catalog.jsonpackages/genui/a2ui/src/catalog/Image/index.tsxpackages/genui/a2ui/src/catalog/List/catalog.jsonpackages/genui/a2ui/src/catalog/List/index.tsxpackages/genui/a2ui/src/catalog/RadioGroup/catalog.jsonpackages/genui/a2ui/src/catalog/RadioGroup/index.tsxpackages/genui/a2ui/src/catalog/Row/catalog.jsonpackages/genui/a2ui/src/catalog/Row/index.tsxpackages/genui/a2ui/src/catalog/Text/catalog.jsonpackages/genui/a2ui/src/catalog/Text/index.tsxpackages/genui/a2ui/tools/catalog_generator.tspackages/genui/a2ui/turbo.json
💤 Files with no reviewable changes (10)
- packages/genui/a2ui/src/catalog/Card/catalog.json
- packages/genui/a2ui/src/catalog/Text/catalog.json
- packages/genui/a2ui/src/catalog/Column/catalog.json
- packages/genui/a2ui/src/catalog/RadioGroup/catalog.json
- packages/genui/a2ui/src/catalog/List/catalog.json
- packages/genui/a2ui/src/catalog/CheckBox/catalog.json
- packages/genui/a2ui/src/catalog/Row/catalog.json
- packages/genui/a2ui/src/catalog/Divider/catalog.json
- packages/genui/a2ui/src/catalog/Button/catalog.json
- packages/genui/a2ui/src/catalog/Image/catalog.json
✅ Files skipped from review due to trivial changes (5)
- .changeset/seven-llamas-stare.md
- eslint.config.js
- biome.jsonc
- packages/genui/a2ui/turbo.json
- packages/genui/a2ui/src/catalog/Image/index.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/genui/a2ui/src/catalog/CheckBox/index.tsx
- packages/genui/a2ui/src/catalog/Divider/index.tsx
- packages/genui/a2ui/src/catalog/List/index.tsx
- packages/genui/a2ui/src/catalog/Button/index.tsx
- packages/genui/a2ui/src/catalog/RadioGroup/index.tsx
- packages/genui/a2ui/src/catalog/Column/index.tsx
- packages/genui/a2ui/src/catalog/Card/index.tsx
- packages/genui/a2ui/package.json
…ed TypeScript generator script
Summary by CodeRabbit
Checklist