Convert Date instances to numbers in minimum/maximum
#5351
Conversation
|
Unfortunately this would break any code that's expecting these values to be some kind of numeric value. I think the better solution would be to convert the |
|
Oh, okay. That's fine too, I think, since a timestamp can be turned into a |
This change aligns the runtime values with typings. Detection of date objects is implemented by checking if `typeof` returns `object`, as the same check is used to initialize `origin`. Furthermore, this changes initialization of test values so that they do not rely on local time zone. (`Date` constructor with multiple numeric parameters interprets them in the local time zone.)
gebsh
changed the title
Date to minimum and maximum in too small/big issuesDate instances to numbers in minimum/maximum
WalkthroughThis PR updates date validation handling in Zod's date checks. Date test constructors were switched from local-time to UTC-based formats to ensure consistent timestamp values. Test validations were refactored to use safeParse with explicit error assertions instead of throw-based testing. Additionally, the checks logic was modified so that when date objects are used as numeric boundaries, their time values are extracted and used in error payloads. These changes normalize how date boundaries are represented in validation error messages without altering the public API. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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: 0
🧹 Nitpick comments (2)
packages/zod/src/v4/core/checks.ts (2)
87-87: Consider a more specific type guard for Date objects.The
typeof def.value === "object"check will match any object, not just Date instances. While this probably works in practice (sinceutil.Numericlikely constrains the types), a more defensive check would prevent potential runtime errors if non-Date objects are passed.Consider one of these alternatives:
- maximum: typeof def.value === "object" ? def.value.getTime() : def.value, + maximum: def.value instanceof Date ? def.value.getTime() : def.value,Or, to handle environments where
instanceofmight be unreliable:- maximum: typeof def.value === "object" ? def.value.getTime() : def.value, + maximum: typeof def.value === "object" && "getTime" in def.value ? def.value.getTime() : def.value,
138-138: Consider a more specific type guard for Date objects.Same concern as line 87 - the
typeof def.value === "object"check is overly broad. For consistency and safety, consider using the same more specific type guard here.- minimum: typeof def.value === "object" ? def.value.getTime() : def.value, + minimum: def.value instanceof Date ? def.value.getTime() : def.value,Or with a method check:
- minimum: typeof def.value === "object" ? def.value.getTime() : def.value, + minimum: typeof def.value === "object" && "getTime" in def.value ? def.value.getTime() : def.value,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/zod/src/v4/classic/tests/date.test.ts(2 hunks)packages/zod/src/v4/core/checks.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,jsx,ts,tsx,mjs,cjs,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce line width of 120 characters via Biome formatting
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ES5-style trailing commas in JavaScript/TypeScript code
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Allow the any type in TypeScript (noExplicitAny off)
Allow non-null assertions in TypeScript (noNonNullAssertion off)
Write TypeScript to pass strict mode with exactOptionalPropertyTypes enabled
Use NodeNext module resolution semantics for imports in TypeScript
Target ES2020 language features in TypeScript source
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CLAUDE.md)
Allow parameter reassignment for performance-sensitive code (noParameterAssign off)
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
**/*.{js,mjs,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,mjs,ts,tsx}: Use .js extensions in import specifiers (e.g., import { z } from "./index.js")
Don’t use require(); use ESM import statements
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
**/*.{js,mjs,cjs,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/guidelines.mdc)
Do not leave log statements (e.g., console.log, debugger) in tests or production code
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
packages/zod/src/v4/core/{schemas.ts,checks.ts}
📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)
When adding issues, push well-formed payload.issues entries including code, expected (when applicable), input, inst, and optional path/message/continue
Files:
packages/zod/src/v4/core/checks.ts
packages/zod/src/v4/core/checks.ts
📄 CodeRabbit inference engine (.cursor/rules/zod-internals.mdc)
Implement checks via inst._zod.check(payload) that only pushes issues when validation fails and sets continue based on def.abort
Files:
packages/zod/src/v4/core/checks.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Write source code in TypeScript (TypeScript-first codebase)
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
packages/zod/**
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Make core Zod library changes in the main package at packages/zod/
Files:
packages/zod/src/v4/core/checks.tspackages/zod/src/v4/classic/tests/date.test.ts
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*
📄 CodeRabbit inference engine (.cursor/rules/testing-guidelines.mdc)
Place all test files under packages/zod/src/v4/classic/tests, packages/zod/src/v4/core/tests, or packages/zod/src/v3/tests
Files:
packages/zod/src/v4/classic/tests/date.test.ts
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/testing-guidelines.mdc)
packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*.test.ts: Test files must use the .test.ts extension (TypeScript), not JavaScript
Use import type for type-only imports in tests (e.g.,import type { ... })
Permanent, regression, API validation, edge-case coverage, and performance benchmark tests must be in the test suite (not play.ts)
Use Vitest as the framework in tests and import from it asimport { expect, test } from "vitest"
Import Zod in tests asimport * as z from "zod/v4"
Write tests with clear, descriptive names and cover both success and failure cases
Keep test suites concise while maintaining adequate coverage
Do not skip tests due to type issues; fix the types instead
Use descriptive file names like string.test.ts, object.test.ts, url-validation.test.ts and group related functionality together
Files:
packages/zod/src/v4/classic/tests/date.test.ts
packages/zod/src/v4/{classic,core}/tests/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/testing-workflow.mdc)
packages/zod/src/v4/{classic,core}/tests/**/*.test.ts: Use Vitest for all testing (Vitest APIs in test files)
Place tests under packages/zod/src/v4/classic/tests/ or packages/zod/src/v4/core/tests/
Name test files with the *.test.ts suffix
Use test() for individual test cases
Use describe() for test groups
Use expect() for assertions
Files:
packages/zod/src/v4/classic/tests/date.test.ts
packages/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/zod-project-guide.mdc)
Use Vitest for tests and place test cases in .test.ts files
Files:
packages/zod/src/v4/classic/tests/date.test.ts
🧠 Learnings (1)
📚 Learning: 2025-10-21T17:26:08.288Z
Learnt from: CR
PR: colinhacks/zod#0
File: .cursor/rules/testing-guidelines.mdc:0-0
Timestamp: 2025-10-21T17:26:08.288Z
Learning: Applies to packages/zod/src/{v4/classic/tests,v4/core/tests,v3/tests}/**/*.test.ts : Permanent, regression, API validation, edge-case coverage, and performance benchmark tests must be in the test suite (not play.ts)
Applied to files:
packages/zod/src/v4/classic/tests/date.test.ts
🔇 Additional comments (3)
packages/zod/src/v4/classic/tests/date.test.ts (3)
5-7: Nice use of UTC constructors for consistent test dates.Using
Date.UTC()ensures the benchmark dates have consistent timestamp values regardless of the local timezone where tests run. This makes the test assertions more reliable across different environments.
20-36: Good test coverage for the timestamp conversion.The test properly validates that
minimumis now a numeric timestamp (1667606400000) instead of a Date object. UsingsafeParsewith explicit error assertions is cleaner than throw-based testing, and the inline snapshot makes the expected behavior crystal clear.
38-54: Solid test coverage for max validation.Mirrors the min test nicely. The snapshot confirms
maximumis correctly converted to a timestamp (1667606400000), validating that the PR objective is achieved - Date boundaries are now represented as numeric timestamps in error payloads.
|
@colinhacks Updated the PR to convert |
2 participants
Fixes #5350.