fix(core): Improves our schema to JSON Schema conversion (fix for zod 4) #2483
Conversation
🦋 Changeset detectedLatest commit: 92cb7a3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
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 |
WalkthroughThe PR removes explicit converter initialization calls from CLI workers and refactors the schema-to-json package API and internals. ConversionOptions now exposes useReferences; ConversionResult no longer includes schemaType. New exports canConvertSchema and isZodSchema were added; initializeSchemaConverters, areConvertersInitialized, and detectSchemaType were removed. Runtime detection and dispatch for Zod (v3/v4), Yup, Effect, and TypeBox were implemented. package.json dependencies updated (zod and effect moved to runtime, other version adjustments). Tests were updated to the new API and to cover Zod v3 and v4. A changeset for a patch release was added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/schema-to-json/src/index.ts (1)
46-57: Calling built-intoJsonSchema()before Zod detection dropsuseReferencessupport for Zod 4.If Zod 4 exposes a built-in converter, this early return prevents passing options. Prefer handling Zod first so you can honor
useReferences.- // Check if schema has a built-in toJsonSchema method (e.g., ArkType, Zod 4) - if (typeof parser.toJsonSchema === "function") { - try { - const jsonSchema = parser.toJsonSchema(); - return { jsonSchema }; - } catch { - // If toJsonSchema fails, continue to other checks - } - } + // Prefer library-specific paths first so options (e.g. useReferences) are honored. + // Built-in converters (e.g. ArkType) are handled later.Then, after the Zod/Yup/Effect/TypeBox branches, add a final built-in check:
// Fallback: generic built-in conversion (e.g., ArkType) if (typeof (parser as any).toJsonSchema === "function") { try { return { jsonSchema: (parser as any).toJsonSchema() }; } catch {} }
🧹 Nitpick comments (8)
packages/schema-to-json/src/index.ts (4)
155-161: Make Yup converter optional to align with “only if available at runtime.”-function convertYupSchema(schema: any): JSONSchema | undefined { - try { - return convertSchema(schema) as JSONSchema; - } catch { - return undefined; - } -} +function convertYupSchema(schema: any): JSONSchema | undefined { + const convertSchema = getYupConvertSchema(); + if (!convertSchema) return undefined; + try { + return convertSchema(schema) as JSONSchema; + } catch { + return undefined; + } +}
167-173: Make Effect converter optional to avoid forcingeffectinto all consumers.-function convertEffectSchema(schema: any): JSONSchema | undefined { - try { - return EffectJSONSchema.make(schema) as JSONSchema; - } catch { - return undefined; - } -} +function convertEffectSchema(schema: any): JSONSchema | undefined { + const make = getEffectJSONSchemaMake(); + if (!make) return undefined; + try { + return make(schema) as JSONSchema; + } catch { + return undefined; + } +}
101-108:canConvertSchemaperforms a full conversion; consider a cheap predicate.For large schemas this adds overhead. Optional: gate with lightweight detection (Zod/Yup/Effect/TypeBox/ArkType heuristics) before attempting conversion.
46-53: Minor: comment accuracy.Zod v4 doesn’t expose an instance
toJsonSchema; it usestoJSONSchemaon the module. Update the comment to avoid confusion.packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
201-202: Pass useReferences to reduce schema size and handle cycles.Forward
{ useReferences: true }so large/recursive Zod schemas (esp. v4) emit$refs instead of duplicating subtrees.Apply this diff:
- const result = schemaToJsonSchema(schema); + const result = schemaToJsonSchema(schema, { useReferences: true });packages/schema-to-json/tests/index.test.ts (3)
4-5: Remove ts-ignore and alias ArkType import to avoid keyword conflict.Using
// @ts-ignorehides real errors. Alias the runtimetypeimport and update call sites.Apply this diff:
-// @ts-ignore -import { type } from "arktype"; +import { type as arkType } from "arktype"; @@ - const schema = type({ + const schema = arkType({ name: "string", age: "number", active: "boolean", }); @@ - const schema = type({ + const schema = arkType({ id: "string", "description?": "string", "tags?": "string[]", }); @@ - expect(canConvertSchema(type("string"))).toBe(true); + expect(canConvertSchema(arkType("string"))).toBe(true);Also applies to: 134-139, 148-153, 262-263
75-85: Rename test to reflect new option and assert refs are emitted.Title mentions “name option” but the code uses
useReferences. Tighten the expectation to guard regressions.Apply this diff:
- it("should handle Zod schema with name option", () => { + it("should generate $ref when useReferences is true", () => { @@ - expect(result).toBeDefined(); - expect(result?.jsonSchema).toBeDefined(); - // The exact structure depends on zod-to-json-schema implementation + expect(result).toBeDefined(); + expect(result?.jsonSchema).toBeDefined(); + const json = JSON.stringify(result?.jsonSchema); + expect(json.includes('"$ref"') || json.includes('"$defs"')).toBe(true);
260-265: Include a Zod v4 case in canConvertSchema coverage.You already cover Zod v3; add a quick v4 check to prevent future regressions.
Apply this diff:
- expect(canConvertSchema(z3.string())).toBe(true); + expect(canConvertSchema(z3.string())).toBe(true); + expect(canConvertSchema(z4.string())).toBe(true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/hello-world/src/trigger/jsonSchema.tsis excluded by!references/**
📒 Files selected for processing (6)
.changeset/angry-files-yawn.md(1 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts(1 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts(1 hunks)packages/schema-to-json/package.json(2 hunks)packages/schema-to-json/src/index.ts(3 hunks)packages/schema-to-json/tests/index.test.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/entryPoints/dev-index-worker.tspackages/cli-v3/src/entryPoints/managed-index-worker.tspackages/schema-to-json/src/index.tspackages/schema-to-json/tests/index.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
packages/schema-to-json/tests/index.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests must use Vitest
Tests should avoid mocks or stubs and use helpers from @internal/testcontainers when Redis or Postgres are needed
Test files live beside the files under test and should use descriptive describe and it blocks
Files:
packages/schema-to-json/tests/index.test.ts
🧠 Learnings (3)
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Import Trigger.dev APIs from "trigger.dev/sdk/v3" when writing tasks or related utilities
Applied to files:
packages/cli-v3/src/entryPoints/dev-index-worker.tspackages/cli-v3/src/entryPoints/managed-index-worker.tspackages/schema-to-json/package.json
📚 Learning: 2025-08-18T10:07:17.368Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-08-18T10:07:17.368Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use schemaTask({ schema, run, ... }) to validate payloads when input validation is required
Applied to files:
packages/cli-v3/src/entryPoints/dev-index-worker.tspackages/cli-v3/src/entryPoints/managed-index-worker.tspackages/schema-to-json/package.json
📚 Learning: 2025-07-18T17:49:24.468Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp
Applied to files:
packages/schema-to-json/package.json
🧬 Code graph analysis (2)
packages/schema-to-json/src/index.ts (1)
packages/core/src/v3/types/jsonSchema.ts (1)
JSONSchema(5-67)
packages/schema-to-json/tests/index.test.ts (1)
packages/schema-to-json/src/index.ts (2)
schemaToJsonSchema(40-99)canConvertSchema(104-107)
⏰ 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). (23)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/cli-v3/src/entryPoints/managed-index-worker.ts (1)
21-21: Remove suggestion to useuseReferences: true. schemaToJsonSchema’s API (v0.0.0-prerelease-20250808093854) only acceptsnameandadditionalPropertiesoptions; there is nouseReferencesflag.Likely an incorrect or invalid review comment.
packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
21-21: Removal of explicit converter init looks good.Leaning on runtime detection via
schemaToJsonSchemakeeps the worker simpler and avoids hidden global state.
| "trigger.dev": patch | ||
| --- | ||
|
|
||
| Improves our schema to JSON Schema conversion, fixes zod 4 and a few other schema libraries, also correctly sets the dependencies |
There was a problem hiding this comment.
Polish the release note (grammar and capitalization).
Capitalize “Zod” and tighten the sentence.
-Improves our schema to JSON Schema conversion, fixes zod 4 and a few other schema libraries, also correctly sets the dependencies
+Improve schema-to-JSON Schema conversion, fix Zod v4 compatibility and a few other schema libraries, and correct dependency setup.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Improves our schema to JSON Schema conversion, fixes zod 4 and a few other schema libraries, also correctly sets the dependencies | |
| Improve schema-to-JSON Schema conversion, fix Zod v4 compatibility and a few other schema libraries, and correct dependency setup. |
🤖 Prompt for AI Agents
In .changeset/angry-files-yawn.md around line 5, the release note sentence needs
capitalization and tightening: change “zod 4” to “Zod 4” and make the sentence
more concise and parallel (for example: “Improves our schema-to-JSON-Schema
conversion, fixes Zod 4 and a few other schema libraries, and correctly sets the
dependencies”). Update the single line accordingly to match grammar and
capitalization.
| "@trigger.dev/core": "workspace:*", | ||
| "zod-to-json-schema": "^3.24.5", | ||
| "@sodaru/yup-to-json-schema": "^2.0.1" | ||
| "zod-to-json-schema": "^3.24.0", | ||
| "@sodaru/yup-to-json-schema": "^2", | ||
| "zod": "3.25.76", | ||
| "effect": "^3" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard runtime coupling to Zod/Effect; rely on peers + lazy import.
src/index.ts currently imports zod/v4 and effect at top-level. Keeping them as direct deps forces consumers to install/bundle them even if unused and causes conflicts (notably zod/v4 when only Zod v3 is present). Prefer optional peer deps + lazy loading in code.
Proposed dependency adjustments (paired with the code changes suggested in src/index.ts):
"dependencies": {
"@trigger.dev/core": "workspace:*",
"zod-to-json-schema": "^3.24.0",
"@sodaru/yup-to-json-schema": "^2",
- "zod": "3.25.76",
- "effect": "^3"
+ // zod and effect are resolved at runtime if present (peer deps)
},You already declare both in peerDependencies (optional). After we lazy-load them in code, this will eliminate duplicate/forced installs. I can send a follow-up PR diff to keep commas/formatting correct across the JSON.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/schema-to-json/package.json around lines 44 to 49, the package
currently hard-declares zod and effect as runtime dependencies which forces
consumers to install/bundle specific versions; remove zod and effect from
"dependencies" (leave them as optional peerDependencies), then update
src/index.ts to lazy-load them via dynamic import (e.g., await import('zod') /
await import('effect') or require at call-time), add clear runtime checks that
throw informative errors if a needed peer is missing, and ensure the code paths
that don’t need Zod/Effect never import them so consumers who don’t use those
features aren’t forced to install those packages.
| @@ -66,7 +66,7 @@ | |||
| "@sinclair/typebox": ">=0.34.30", | |||
| "valibot": ">=0.41.0", | |||
| "yup": ">=1.0.0", | |||
| "zod": "^3.24.1 || ^4.0.0" | |||
| "zod": "^3.25.76 || ^4" | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Peer range looks good; conflict risk remains if we also list zod as a direct dep.
With "zod": "^3.25.76 || ^4" as a peer, drop the direct dependency to prevent dual-installs and resolution of zod/v4 failing when only v3 is present.
🤖 Prompt for AI Agents
In packages/schema-to-json/package.json around line 69, the package declares
"zod" as a peer dependency ("^3.25.76 || ^4") but still lists zod as a direct
dependency which can cause duplicate installs and v4 resolution failures when
only v3 is present; remove zod from the direct "dependencies" (or
"devDependencies" if present) so it is only a peerDependency, update
package.json accordingly, and run a quick install/test to confirm no code paths
relied on the direct import (if any test/build scripts require zod during
development, instead add it to devDependencies in the consuming workspace only).
| import { zodToJsonSchema } from "zod-to-json-schema"; | ||
| import * as z4 from "zod/v4"; | ||
| import { convertSchema } from "@sodaru/yup-to-json-schema"; | ||
| import { JSONSchema as EffectJSONSchema } from "effect"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Top-level imports make optional converters mandatory; zod/v4 will crash when only Zod v3 is installed.
This defeats the “only if available at runtime” goal and can break consumers lacking Zod v4. Make these lazy/optional.
-import { zodToJsonSchema } from "zod-to-json-schema";
-import * as z4 from "zod/v4";
-import { convertSchema } from "@sodaru/yup-to-json-schema";
-import { JSONSchema as EffectJSONSchema } from "effect";
+import { createRequire } from "node:module";
+const requireOptional =
+ // eslint-disable-next-line @typescript-eslint/no-implied-eval
+ ((r => (id: string) => { try { return r(id); } catch { return undefined; } })
+ // @ts-expect-error: available in Node builds; CJS build will polyfill via tshy
+ )(typeof require !== "undefined" ? require : (await import("node:module")).createRequire(import.meta.url));
+
+let _z4: undefined | (typeof import("zod/v4"));
+const getZ4 = () => (_z4 ??= requireOptional("zod/v4"));
+
+let _zodToJsonSchema: undefined | (typeof import("zod-to-json-schema"))["zodToJsonSchema"];
+const getZodToJsonSchema = () => (_zodToJsonSchema ??= requireOptional("zod-to-json-schema")?.zodToJsonSchema);
+
+let _yupConvertSchema: undefined | (typeof import("@sodaru/yup-to-json-schema"))["convertSchema"];
+const getYupConvertSchema = () => (_yupConvertSchema ??= requireOptional("@sodaru/yup-to-json-schema")?.convertSchema);
+
+let _effectJSONSchemaMake: undefined | ((schema: unknown) => unknown);
+const getEffectJSONSchemaMake = () => (_effectJSONSchemaMake ??= requireOptional("effect")?.JSONSchema?.make);If you prefer, I can refactor to pure dynamic import() with small async wrappers and keep the public API sync by returning undefined when modules aren’t present.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/schema-to-json/src/index.ts around lines 3-6, avoid top-level
imports of optional converters (e.g., "zod/v4") which crash when those versions
are absent; change to lazy runtime resolution by replacing the direct imports
with dynamic imports inside the functions that need them (use try/catch around
import() and return undefined or fallback behavior if the module is not
present), ensure any exported conversion functions gracefully handle a missing
module by returning undefined or an error value synchronously, and update
typings to use broad types or conditional types so the module compiles without
the optional dependency.
| function convertZod3Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined { | ||
| const useReferences = options?.useReferences ?? false; | ||
|
|
||
| return zodToJsonSchema(schema, { | ||
| $refStrategy: useReferences ? "root" : "none", | ||
| }) as JSONSchema; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard Zod v3 converter behind optional import to avoid hard coupling.
-function convertZod3Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined {
- const useReferences = options?.useReferences ?? false;
- return zodToJsonSchema(schema, {
- $refStrategy: useReferences ? "root" : "none",
- }) as JSONSchema;
-}
+function convertZod3Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined {
+ const zodToJsonSchema = getZodToJsonSchema();
+ if (!zodToJsonSchema) return undefined;
+ const useReferences = options?.useReferences ?? false;
+ return zodToJsonSchema(schema, { $refStrategy: useReferences ? "root" : "none" }) as JSONSchema;
+}Committable suggestion skipped: line range outside the PR's diff.
| function convertZod4Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined { | ||
| const useReferences = options?.useReferences ?? false; | ||
|
|
||
| return z4.toJSONSchema(schema, { | ||
| target: "draft-7", | ||
| io: "output", | ||
| reused: useReferences ? "ref" : "inline", | ||
| }) as JSONSchema; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard Zod v4 path behind optional import.
Avoid requiring Zod v4 in environments with only Zod v3 installed.
-function convertZod4Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined {
- const useReferences = options?.useReferences ?? false;
- return z4.toJSONSchema(schema, {
+function convertZod4Schema(schema: any, options?: ConversionOptions): JSONSchema | undefined {
+ const z4 = getZ4();
+ if (!z4?.toJSONSchema) return undefined;
+ const useReferences = options?.useReferences ?? false;
+ return z4.toJSONSchema(schema, {
target: "draft-7",
io: "output",
reused: useReferences ? "ref" : "inline",
}) as JSONSchema;
}Committable suggestion skipped: line range outside the PR's diff.
ericallam
force-pushed
the
fix/schema-to-json-improvements
branch
from
e2c8a8e to
92cb7a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/schema-to-json/tests/index.test.ts (5)
4-6: Remove @ts-ignore by aliasing ArkType’stypeexportImporting a binding literally named
typeforces a@ts-ignore. Alias it and drop the suppression.-// @ts-ignore -import { type } from "arktype"; +import { type as arkType } from "arktype"; @@ - const schema = type({ + const schema = arkType({ name: "string", age: "number", active: "boolean", }); @@ - const schema = type({ + const schema = arkType({ id: "string", "description?": "string", "tags?": "string[]", });Also applies to: 134-139, 148-152
75-85: Rename test: it doesn’t set a “name” optionThe body only enables
{ useReferences: true }; no “name” option is provided. Rename for clarity.- it("should handle Zod schema with name option", () => { + it("should handle Zod schema with useReferences option", () => {
164-181: Optional: strengthen Effect optional-fields assertionsConsider asserting required keys to catch regressions (optional/low-risk).
expect(result).toBeDefined(); expect(result?.jsonSchema).toMatchObject({ type: "object", properties: { name: { type: "string" }, age: { type: "number" }, active: { type: "boolean" }, }, - required: ["name", "age", "active"], + required: ["name", "age", "active"], }); @@ expect(result).toBeDefined(); expect(result?.jsonSchema).toBeDefined(); expect(result?.jsonSchema.type).toBe("object"); + // Optional: only non-optional fields should be required + expect(result?.jsonSchema).toMatchObject({ required: ["id"] });Also applies to: 185-196
260-265: Also assert canConvertSchema for Zod v4You exercise Zod v3 here; include a Zod v4 primitive to cover both code paths.
- expect(canConvertSchema(z3.string())).toBe(true); + expect(canConvertSchema(z3.string())).toBe(true); + expect(canConvertSchema(z4.string())).toBe(true);
9-11: Nit: explicitly import vitest globals (optional)If TS isn’t configured with vitest global types, add explicit imports to avoid ambient-typing drift.
+import { describe, it, expect } from "vitest";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/hello-world/src/trigger/jsonSchema.tsis excluded by!references/**
📒 Files selected for processing (6)
.changeset/angry-files-yawn.md(1 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts(1 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts(1 hunks)packages/schema-to-json/package.json(2 hunks)packages/schema-to-json/src/index.ts(3 hunks)packages/schema-to-json/tests/index.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cli-v3/src/entryPoints/dev-index-worker.ts
- packages/schema-to-json/src/index.ts
- .changeset/angry-files-yawn.md
- packages/cli-v3/src/entryPoints/managed-index-worker.ts
- packages/schema-to-json/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/schema-to-json/tests/index.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
packages/schema-to-json/tests/index.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests must use Vitest
Tests should avoid mocks or stubs and use helpers from @internal/testcontainers when Redis or Postgres are needed
Test files live beside the files under test and should use descriptive describe and it blocks
Files:
packages/schema-to-json/tests/index.test.ts
🧬 Code graph analysis (1)
packages/schema-to-json/tests/index.test.ts (1)
packages/schema-to-json/src/index.ts (2)
schemaToJsonSchema(40-99)canConvertSchema(104-107)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/schema-to-json/tests/index.test.ts (3)
1-2: LGTM on dual Zod imports and public API usageImporting v3 and v4 side-by-side and targeting
schemaToJsonSchema/canConvertSchemadirectly aligns with the new API surface.Also applies to: 8-8
13-31: Solid, focused assertions across schema familiesThese tests validate core shapes, formats, and required arrays without over-coupling to converter internals. Nice.
Also applies to: 55-73, 90-108, 110-129, 201-219, 221-240, 244-255
33-51: Confirmed — no change required: use z.email()
z.email() is the preferred top-level email constructor in Zod v4; z.string().email() is deprecated but still supported. The test is correct as written.
No description provided.