fix(api): safeextend when attaching variable schemas#838
Conversation
The .safeExtend() method works similarly to .extend(), but it won't let you overwrite an existing property with a non-assignable schema. In other words, the result of .safeExtend() will have an inferred type that extends the original (in the TypeScript sense). Use .safeExtend() to extend schemas that contain refinements. (Regular .extend() will throw an error when used on schemas with refinements.)
There was a problem hiding this comment.
Pull request overview
Updates the variable-resolver schema attachment logic to use Zod’s .safeExtend()/safeExtend() when rebuilding object shapes, improving compatibility with schemas that include refinements and preserving “type extends original” semantics.
Changes:
- Replace
extend(...)withsafeExtend(...)forZodMiniObjectshape rebuilding. - Replace
.extend(...)with.safeExtend(...)forZodObjectshape rebuilding (introducing anas anycast to satisfy typing).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| acc.extend({ | ||
| [key]: attachVariableResolver(variablesRegistry, parser), | ||
| acc.safeExtend({ | ||
| [key]: attachVariableResolver(variablesRegistry, parser) as any, |
There was a problem hiding this comment.
as any on the value passed to acc.safeExtend() defeats the main benefit of safeExtend (type-level guarantee that the new schema is assignable to the existing property type). It would be better to preserve typing by casting to a narrower type (e.g., based on the existing parser type) or by constructing a typed extension shape first and calling safeExtend once, so TypeScript can keep key/value relationships without falling back to any.
| [key]: attachVariableResolver(variablesRegistry, parser) as any, | |
| [key]: attachVariableResolver(variablesRegistry, parser) as $ZodType, |
| (acc, [key, parser]) => | ||
| extend(acc, { | ||
| safeExtend(acc, { | ||
| [key]: attachVariableResolver(variablesRegistry, parser), | ||
| }), | ||
| objectParser, |
There was a problem hiding this comment.
This change is meant to fix extending object schemas that include refinements, but current tests only cover .refine() on a primitive (z.string().refine(...)) and don't exercise objectVariableResolverParser on a refined object schema (e.g., z.object({...}).refine(...) or superRefine). Please add a regression test that calls Data.create().usePayload(...) with a refined object payload and uses variables within nested properties, ensuring addRules() no longer throws during schema attachment.
The .safeExtend() method works similarly to .extend(), but it won't let you overwrite an existing property with a non-assignable schema. In other words, the result of .safeExtend() will have an inferred type that extends the original (in the TypeScript sense).
Use .safeExtend() to extend schemas that contain refinements. (Regular .extend() will throw an error when used on schemas with refinements.)