-
Notifications
You must be signed in to change notification settings - Fork 116
fix(react-sdk): improve Zod compatibility and schema test coverage #1620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new cross-version Zod detection is directionally good, but isZodSchema()/isZod4Schema() rely on internal markers in ways that can still produce false positives and route callers into zod4ToJSONSchema incorrectly. Importing z from "zod/v4" in json-schema.ts is a runtime compatibility risk given the supported peer range and varying subpath export behavior. One of the new tests in schema.test.ts is mislabeled and does not assert the behavior it claims, which should be corrected to keep the suite trustworthy.
Additional notes (3)
- Compatibility |
react-sdk/src/schema/json-schema.ts:1-5
react-sdknow importszfrom"zod/v4"directly. Given this package supportszod@^3.25.76 || ~4.0.0, importing"zod/v4"at runtime assumes the installedzodpackage exposes that subpath. While Zod 3.25.x historically shipped v4 internals underzod/v4, this is brittle across patch releases and bundlers (some setups restrict package subpath exports).
If this file is only used for validating JSON Schema shapes internally, consider removing the runtime dependency on zod/v4 entirely and implementing the validator without Zod (or use a small hand-written checker). Otherwise, consider importing from "zod" and branching per detected API, or documenting why "zod/v4" is guaranteed across the supported range.
- Compatibility |
react-sdk/src/schema/zod.ts:1-1
zod.tsalready usesimport { JSONSchema7 } from "json-schema";(value import). In test files you switched toimport type { JSONSchema7 } ...which avoids pulling runtime deps; that’s good. Here though, the import is only used for type assertions (as JSONSchema7). This should be a type-only import to avoid any potential runtime overhead/bundler side-effects.
Also, you’re casting the result of zod4ToJSONSchema/zodToJsonSchema to JSONSchema7, but those converters may produce schemas outside draft-07 (e.g., $defs, newer keywords). The cast hides that mismatch and can lead to downstream code assuming draft-07 semantics incorrectly.
- Maintainability |
react-sdk/src/schema/standard-schema.ts:20-45
isStandardSchemanow relies on duck typing, which is correct for cross-Zod compatibility. However, it only checks thatvalidateis a function, not that calling it returns something consistent with the spec (sync/async result object). That’s fine for a type guard, but be careful where this is used: treating any object with avalidatefunction as a Standard Schema can allow non-schema objects to flow into conversion paths.
If this guard is used to decide whether to attempt JSON Schema conversion or validation, consider tightening the check slightly (without invoking validate) by verifying additional known keys (if any) or vendor allowlist where appropriate.
Summary of changes
What changed
Dependency & packaging updates
- Updated
react-sdkZod peer range to^3.25.76 || ~4.0.0and madezodnon-optional inpeerDependenciesMeta. - Removed the internal
zodInternalAliasdependency/workaround:- Deleted
react-sdk/src/schema/alias.ts - Removed
zodInternalAliasfromreact-sdk/package.jsonandpackage-lock.json.
- Deleted
Zod / Standard Schema compatibility work
- Rewrote
isStandardSchema()to use duck-typing instead of Zod validation (safeParse). - Updated Zod v4 function detection and extraction logic in
react-sdk/src/schema/zod.ts:- Fixed
isZod4FunctionSchema()schema.defcheck. - Expanded
isZod4Schema()/isZodSchema()detection to handle v4.0.x vs v4.1.x differences. - Updated
getZodFunctionReturns()to read fromschema.def?.output(v4.0.x) orschema._zod?.def?.output(v4.1.x). - Cast JSON Schema outputs to
JSONSchema7.
- Fixed
Tests & coverage
- Added new
standard-schema.test.tscovering Standard Schema detection for Zod 3/4 and custom implementations. - Expanded tests for:
- JSON Schema tuple handling in
schema.test.ts assertNoRecordSchemaedge cases and conditional keywords invalidate.test.ts- Zod schema detection and Zod 4 function arg/return extraction in
zod.test.ts.
- JSON Schema tuple handling in
Repo guidance
- Updated
AGENTS.mdto require@returnsin JSDoc when adding/editing JSDoc comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main blockers are runtime compatibility and test correctness: importing z from "zod/v4" in json-schema.ts is brittle across consumer environments, and schema.test.ts contains a test whose name contradicts its assertion. Additionally, isZodSchema()’s v4.0 function fallback is still too permissive ("~standard" in ... without vendor validation), and the new JSONSchema7 casts in handleZodSchemaToJson() can hide schema-dialect mismatches. The duck-typed isStandardSchema() is directionally right but may warrant slightly stricter checks if it gates conversion/validation behavior.
Additional notes (1)
- Maintainability |
react-sdk/src/schema/standard-schema.ts:22-44
isStandardSchema()now correctly avoids Zod validation, but it performs anincheck and then indexesobj["~standard"]on anunknownobject.
This is fine at runtime, but the guard is used to assert conformance. Right now it accepts any object with { "~standard": { version: 1, vendor: string, validate: function } }, which is broad enough to let non-schema objects into schema conversion/validation paths. That can produce confusing downstream errors.
If this guard gates meaningful behavior (conversion/validation), consider tightening it slightly without calling validate (e.g., check that validate.length >= 1, or that standard has only expected keys, or vendor allowlisting where appropriate).
Summary of changes
Summary
This PR focuses on cross-version Zod compatibility and schema utility test coverage in react-sdk.
Dependency / packaging updates
- Tightened the Zod peer range to
^3.25.76 || ~4.0.0and removed optionality for thezodpeer. - Removed the
zodInternalAliasworkaround:- Deleted
react-sdk/src/schema/alias.ts - Dropped
zodInternalAliasfromreact-sdk/package.jsonandpackage-lock.json.
- Deleted
Schema detection & conversion changes
isStandardSchema()switched from Zod validation to duck typing to avoid Zod 3 vs 4 runtime incompatibilities.- Updated Zod v4 detection/extraction logic in
react-sdk/src/schema/zod.ts:- Improved v4.0.x vs v4.1.x detection (
defvs_zod). - Fixed v4 function schema detection (
def.type === "function"). - Updated arg/return extraction to support
schema.def?.output ?? schema._zod?.def?.output. - Cast Zod→JSON schema conversion results to
JSONSchema7.
- Improved v4.0.x vs v4.1.x detection (
Tests & coverage
- Added
standard-schema.test.tswith broad Standard Schema detection coverage. - Expanded
schema.test.tswith JSON Schema tuple/prefixItems parameter extraction cases. - Expanded
validate.test.tsto cover conditional keywords (if/then/else,not,oneOf),prefixItems, and edge inputs. - Updated
zod.test.tsto assert Zod 4 function schema argument/return extraction works.
Repo guidance
- Updated
AGENTS.mdto require@returnswhen adding/editing JSDoc comments.
Summary
^3.25.76 || ~4.0.0(drops 4.1 which caused TypeScript issues)zodInternalAliasworkaround that was no longer neededisStandardSchemato use duck typing instead of Zod 4 validation (avoids cross-version compatibility issues)Changes
Zod Compatibility
^3.25.76 || ^4.1.0to^3.25.76 || ~4.0.0zodInternalAliasdev dependency that was used as a workaroundzoda required (not optional) peer dependencySchema Detection Improvements
isStandardSchemato use duck typing instead of Zod 4'sz.object().safeParse(), which was throwing errors when validating Zod 3 schemasisZod4FunctionSchemato properly detect v4.0.x function schemas viadef.type === "function"getZodFunctionArgsandgetZodFunctionReturnsto extract args/returns from Zod 4 function schemasTest Coverage
standard-schema.test.tswith comprehensive coverage (100% statements/branches)validate.tscovering JSON Schema conditional keywords (if/then/else, not, oneOf), prefixItems, and edge casesschema.tscovering toolSchema with JSON Schema tuples and edge caseszod.test.tsto verify Zod 4 function schema extraction works correctlyTest Plan
standard-schema.ts88.88% → 100%,validate.ts82.75% → 96.55%🤖 Generated with Claude Code