feat: enforce zod/json-ness of propSchema prop #276
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
The changes improve schema handling but introduce duplicated checks for Zod schemas and error handling that only logs issues without a clear fallback. By consolidating these checks and improving error handling, the code’s maintainability and clarity can be enhanced.
Summary of changes
Summary of Changes
component-metadata.ts: Updated the type ofpropsSchemato accept both Zod schemas (z.ZodTypeAny) and JSON schemas (JSONSchema7), and deprecated thepropsDefinitionproperty with an added JSDoc@deprecatedannotation.tambo-registry-provider.tsx: Replaced inline props schema serialization with a helper functiongetSerializedProps. This function now handles conversion of props definitions by detecting whether the provided schema is a Zod schema or a JSON schema, with added helper functionsisJSONSchemaandisZodSchema.
| if (propsSchema instanceof ZodSchema || propsSchema instanceof z.ZodObject) { | ||
| return zodToJsonSchema(propsSchema); | ||
| } | ||
|
|
There was a problem hiding this comment.
The check for propsSchema being a Zod schema is duplicated: first using instanceof ZodSchema || instanceof z.ZodObject and then via the isZodSchema helper. This duplication reduces clarity and increases maintenance overhead. Consider consolidating these checks to streamline the logic.
Merge the Zod schema detection into a single check – for instance, always use isZodSchema and then perform the conversion with proper error handling. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| if (isZodSchema(propsSchema)) { | ||
| try { | ||
| return zodToJsonSchema(propsSchema); | ||
| } catch (error) { | ||
| console.error( | ||
| `Error converting ${name} props schema to JSON Schema:`, | ||
| error, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Within the try-catch block for converting a Zod schema (lines 210-218), the catch clause only logs the error without returning any fallback value, which leads to a generic error throw later. This approach might obscure the underlying conversion issue.
Consider either rethrowing the caught error or returning a specific fallback value to make troubleshooting easier. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
No description provided.