feat(temporal-zod): add oRPC support - #70
Conversation
oRPC's ZodToJsonSchemaConverter re-implements Zod → JSON Schema instead of calling z.toJSONSchema(), so it ignores the .meta() metadata temporal-zod attaches and emits a messy anyOf for each z.union-based validator. Add a temporal-zod/orpc entry point exporting temporalJsonSchemaInterceptor, which short-circuits the converter and returns the registered JSON Schema (type/format/pattern) directly. @orpc/zod is an optional peer dependency and only its types are imported, so there is no new runtime dependency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…orpc dep Removes the temporal-zod/orpc subpath export in favour of exporting temporalJsonSchemaInterceptor from the main entry. A type-only import still has to resolve in the shipped .d.ts, so re-exporting the previous @orpc/zod-typed interceptor from the main entry would have broken consumers who don't install @orpc/zod and don't set skipLibCheck (verified: TS2307). Instead the interceptor is now typed structurally and imports nothing from @orpc/* — so temporal-zod drops the optional peer dependency entirely and consumers who don't use oRPC have nothing to resolve. A test asserts the structural type is still assignable to oRPC's real interceptor signature, so drift fails the build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Regenerated lockfile drops the stale @orpc/zod optionalPeer entry left over from the drop-@orpc-dep refactor.
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
WalkthroughAdds an oRPC JSON Schema interceptor for Temporal validators and widens exported serialization patterns to cover ChangesTemporal pattern contracts
oRPC integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ZodToJsonSchemaConverter
participant temporalJsonSchemaInterceptor
participant ZodSchema
participant TemporalValue
ZodToJsonSchemaConverter->>temporalJsonSchemaInterceptor: convert Temporal Zod schema
temporalJsonSchemaInterceptor->>ZodSchema: read global registry metadata
temporalJsonSchemaInterceptor-->>ZodToJsonSchemaConverter: return string schema with format/pattern
TemporalValue->>ZodSchema: parse toJSON() wire string
ZodSchema-->>TemporalValue: return Temporal instance
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/temporal-zod/src/orpc.ts (1)
101-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider explicit field extraction instead of double cast.
The
as unknown as TemporalJsonSchemadouble cast on line 109 bypasses type checking entirely. While pragmatically sound (temporal-zod controls the metadata), an explicit extraction of known fields would make the type contract self-documenting and catch metadata shape regressions at compile time.This is optional — the current approach is more future-proof if temporal-zod ever adds new metadata fields that should pass through to the JSON Schema.
♻️ Optional: explicit field extraction
export const temporalJsonSchemaInterceptor: TemporalSchemaInterceptor = ( options, ) => { const meta = z.globalRegistry.get(options.schema) as | (Record<string, unknown> & { type?: unknown }) | undefined; if (meta && typeof meta.type === "string") { - const { id: _id, ...jsonSchema } = meta; - return [true, jsonSchema as unknown as TemporalJsonSchema]; + const { id: _id, type, ...rest } = meta; + const jsonSchema: TemporalJsonSchema = { type, ...rest } as TemporalJsonSchema; + return [true, jsonSchema]; } return options.next(); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/temporal-zod/src/orpc.ts` around lines 101 - 112, Replace the double cast in temporalJsonSchemaInterceptor with explicit extraction and construction of the known TemporalJsonSchema fields from meta, preserving the id omission and next() fallback while allowing compile-time validation of the metadata shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/temporal-zod/src/orpc.ts`:
- Around line 101-112: Replace the double cast in temporalJsonSchemaInterceptor
with explicit extraction and construction of the known TemporalJsonSchema fields
from meta, preserving the id omission and next() fallback while allowing
compile-time validation of the metadata shape.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e6ab239-ce85-446c-9358-287649029312
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.changeset/orpc-support.mdpackages/temporal-zod/README.mdpackages/temporal-zod/package.jsonpackages/temporal-zod/src/index.tspackages/temporal-zod/src/orpc.test.tspackages/temporal-zod/src/orpc.ts
…poral types" This reverts commit 307aa97. Dropping the custom RPC serializer approach in favour of the direction taken in #70: Temporal values stay plain ISO strings on the wire, with temporal-zod validators reviving them on parse. Only the format-temporal republish changeset remains on this branch.
… tests The interceptor is metadata-driven rather than per-type, but the tests only exercised Instant, PlainDate, PlainTime and Duration. Extend them to all eight types, in both the coercing and instance variants, asserting a clean string schema (no anyOf, no $ref, no dangling id), the advertised pattern and format, and that the bare converter is still broken without it. Also add the wire round trip each type goes through in practice: the server serializes with toJSON(), the advertised pattern accepts that string, and the client revives it by parsing with the same validator. ZonedDateTime gets dedicated cases for its bracketed IANA annotation, a non-ISO calendar suffix, and a DST fall-back instant. Record one pre-existing mismatch as characterization tests: the validators parse the [u-ca=...] annotation that toJSON() emits for a non-ISO calendar, but the exported patterns reject it, so the published JSON Schema is narrower than what the server emits and the client accepts. Same for extended (BCE) years. These patterns predate oRPC support and are unchanged here; the tests document the gap where it becomes a published contract. Formatted with biome, this branch's formatter (master's oxfmt pre-commit hook does not resolve here).
|
Audited this branch for per-type completeness and pushed 58b4bbc extending the tests from 9 to 45. The interceptor itself is sound. It's metadata-driven rather than per-type, and that generality holds: all eight Temporal types carry One pre-existing mismatch worth a decision. The validators parse via
Note Nothing breaks at runtime, since oRPC validates with the Zod schema rather than the emitted JSON Schema. But anything enforcing the advertised contract — generated clients, an API gateway, ajv over the OpenAPI document — would reject values that work end to end.
These patterns predate oRPC support and are untouched here — this PR is just where they become a published contract, which is why I documented rather than changed them. I left the mismatch as clearly-marked characterization tests ( Checks on this branch: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/temporal-zod/src/orpc.test.ts`:
- Around line 328-405: The exported Temporal patterns used by the interceptor
must match the wire format accepted by the validators. Update
PLAIN_DATE_PATTERN, PLAIN_DATE_TIME_PATTERN, PLAIN_YEAR_MONTH_PATTERN, and
PLAIN_MONTH_DAY_PATTERN to support calendar annotations and extended BCE years,
and tighten ZONED_DATE_TIME_PATTERN so malformed time-zone annotations are
rejected; then replace the characterization block with contract tests covering
valid non-ISO/BCE values and invalid annotations.
- Around line 272-277: Update the test “a value that is already an instance
passes through parse unchanged” to assert reference identity between parsed and
value, while retaining the existing toJSON comparison if useful.
🪄 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 Plus
Run ID: ec31dc06-aa15-48e7-aaed-12ac158cfac6
📒 Files selected for processing (1)
packages/temporal-zod/src/orpc.test.ts
| test("a value that is already an instance passes through parse unchanged", () => { | ||
| for (const { coerce, value } of ALL_TEMPORAL_TYPES) { | ||
| const parsed = coerce.parse(value) as { toJSON: () => string }; | ||
|
|
||
| expect(parsed.toJSON()).toBe(value.toJSON()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert instance identity for the pass-through contract.
A clone or reconstructed value passes the current toJSON() comparison. Assert reference identity to verify the stated behavior.
Proposed fix
const parsed = coerce.parse(value) as { toJSON: () => string };
- expect(parsed.toJSON()).toBe(value.toJSON());
+ expect(parsed).toBe(value);📝 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.
| test("a value that is already an instance passes through parse unchanged", () => { | |
| for (const { coerce, value } of ALL_TEMPORAL_TYPES) { | |
| const parsed = coerce.parse(value) as { toJSON: () => string }; | |
| expect(parsed.toJSON()).toBe(value.toJSON()); | |
| } | |
| test("a value that is already an instance passes through parse unchanged", () => { | |
| for (const { coerce, value } of ALL_TEMPORAL_TYPES) { | |
| const parsed = coerce.parse(value) as { toJSON: () => string }; | |
| expect(parsed).toBe(value); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/temporal-zod/src/orpc.test.ts` around lines 272 - 277, Update the
test “a value that is already an instance passes through parse unchanged” to
assert reference identity between parsed and value, while retaining the existing
toJSON comparison if useful.
…d patterns Covering all eight Temporal types against oRPC's real converter flushed out a mismatch between what the validators parse and what they advertise. The validators defer to Temporal.X.from(), which accepts strings the exported patterns rejected. That was survivable while the patterns only fed z.toJSONSchema(), but the interceptor publishes them as an OpenAPI contract, so a generated client or gateway would reject values that work end to end. Widen the patterns to accept: - The [u-ca=...] annotation appended under a non-ISO calendar, for PlainDate, PlainDateTime, PlainYearMonth, PlainMonthDay and ZonedDateTime. - The full reference-date form PlainYearMonth and PlainMonthDay serialize to under a non-ISO calendar (e.g. 2022-12-25[u-ca=hebrew]). The annotation is required in that form, so a bare calendar date is still rejected for both. - Signed six-digit years outside 0000-9999 (e.g. -000753-04-21). ZONED_DATE_TIME_PATTERN no longer ends in \[.+\]: the greedy .+ spanned both bracket groups, so it matched the calendar suffix only by accident and also accepted malformed annotations like [not a time zone!][]. The patterns are now composed from shared internal parts rather than eight hand-copied regexes, so the grammar is stated once. The parts are not exported; the public surface stays the eight *_PATTERN strings. Add explicit converter tests for ZonedDateTime, PlainDateTime, PlainYearMonth and PlainMonthDay alongside the existing four, pattern vectors for the widened forms, and an ajv check that serialized non-ISO values validate against the generated schema. One mismatch is left deliberately: PlainDate also advertises format: "date", which is RFC 3339 full-date and cannot hold an annotation. Dropping the format would degrade every ISO consumer to accommodate a rare one, so it stays, with a test and README note marking the boundary.
|
Follow-up to the audit above — pushed 43a0ab0, which fixes what the expanded coverage flushed out. Explicit converter tests now exist for the four types that lacked them: The patterns did not hold. They rejected strings the validators themselves parse, which was survivable while they only fed
The eight patterns are now composed from shared internal parts instead of hand-copied regexes, so the grammar is stated once. The parts are not exported — the public surface is still the eight One mismatch left deliberately. Verification: existing vectors all keep their verdicts (I checked the widened patterns against every pre-existing valid/invalid case before changing anything), 12 calendars × 5 types round-trip, 7 time zone forms match, and ajv validates serialized non-ISO output against the generated schema. Suite is 177 passing, up from 9 on this file when I started. README's oRPC section now states the eight-type coverage, the ISO-string wire format, and the |
Resolve conflicts in packages/temporal-zod/package.json (take master's oxlint-era layout, keep @orpc/zod devDependency needed by orpc.test.ts) and bun.lock (regenerated with bun install).
Master merged #73 (ponyfill-temporal), which swaps temporal-zod's Temporal import to the ponyfill and drops the direct temporal-polyfill dependency. - Resolve import conflicts in the six base files: keep master's ponyfill-temporal import alongside this branch's iso-pattern-parts imports; migrate orpc.test.ts off temporal-polyfill too. - Preload temporal-polyfill/full/global in bun tests (bunfig.toml): Bun has no native Temporal and the ponyfill's default fallback omits non-ISO calendar data, which the calendar-annotation tests exercise. - Satisfy oxlint x isolatedDeclarations on the exported patterns: template-literal exports need an explicit type (TS9010), so widen with 'as string'; where the interpolated parts are already string-typed, oxlint flags the assertion as unnecessary, so those sites carry a disable comment. - Regenerate bun.lock via bun install.
The root test preload (test-setup.ts) side-effect imports temporal-polyfill/full/global so Bun's tests get the full-ICU build, but only packages/ponyfill-temporal declared the dependency. Under bun's isolated linker — what CI resolves to — temporal-polyfill is not hoisted to the root node_modules, so oxlint's type-aware pass failed with TS2882 on the import.
1 participant
Summary
Adds oRPC support to
temporal-zodby exportingtemporalJsonSchemaInterceptorfrom the main entry point.The problem: oRPC generates OpenAPI with its own
ZodToJsonSchemaConverter(@orpc/zod/zod4), which re-implements the Zod → JSON Schema conversion rather than callingz.toJSONSchema(). It therefore ignores the.meta()JSON Schema metadatatemporal-zodattaches. Because each Temporal validator is az.union([...])underneath, the converter emits a messyanyOfand dropsformat/pattern. oRPC's other extension points don't help: its own registries merge over the structural output (leaving theanyOfin place), and its zod-globalRegistryfallback only readstitle/description/examples, nevertype/format/pattern.The fix: oRPC's
interceptorsare the one mechanism that can fully short-circuit the structural walk.No dependency on oRPC — not even a type-only one
The interceptor is typed structurally, so
temporal-zodimports nothing from@orpc/*.This matters: a type-only import still has to resolve in the shipped
.d.ts. An earlier iteration typed the interceptor viaimport type { ZodToJsonSchemaConverterOptions } from "@orpc/zod/zod4", which was fine at runtime but broke typecheck for consumers who don't install@orpc/zodand don't setskipLibCheck. Verified against a simulated consumer:skipLibCheckimport typefrom@orpc/zodtruefalseTS2307: Cannot find module '@orpc/zod/zod4'So there's no optional peer dep, no subpath export, and nothing to resolve for consumers who don't use oRPC.
A test asserts the structural type is still assignable to oRPC's real interceptor signature, so any upstream drift fails the build.
Changes
temporalJsonSchemaInterceptor(+TemporalSchemaInterceptor/TemporalJsonSchematypes) exported fromtemporal-zodsrc/orpc.test.ts— 9 tests against the real converter, including a control proving the bare converter produces the brokenanyOf, one confirming unrelated consumer.meta()is left untouched, and the oRPC-signature compatibility assertionAll 112 tests pass; build and lint clean.
🤖 Generated with Claude Code
Summary by CodeRabbit
format/pattern.temporalJsonSchemaInterceptorfrom the package top level.*_PATTERNregexes to accept alltoJSON()-emitted variants, and fixedZonedDateTimepattern behavior to prevent bracket-group swallowing.PlainDatestill enforcesformat: "date"for validation.