-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add support for Date type in TypeBox codegen #16
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
This change adds a new `DateTypeHandler` class that can handle the `Date` type in TypeScript. The `DateTypeHandler` is registered in the `TypeBoxTypeHandlers` class, allowing the codegen to generate TypeBox expressions for `Date` types. This enhancement provides native support for the JavaScript `Date` type, making it easier to work with date-related data in the generated validation schemas.
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Date type handler to the TypeBox handler system, wires it into the handler registry for type-reference mapping, updates architecture docs to describe JavaScript built-in type support and handler organization, and introduces tests covering Date handling across aliases, objects, unions, arrays, and function signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Dev
participant TS as TS Source (ts-morph)
participant TH as TypeBoxTypeHandlers
participant DH as DateTypeHandler
participant CG as CodeGen (makeTypeCall)
participant Out as Emitted TypeBox Code
Dev->>TS: Define types using Date (alias/object/union/array/function)
TS->>TH: Visit TypeReferenceNode(Date)
TH->>DH: canHandle(Date)?
DH-->>TH: true
TH->>DH: handle(Date)
DH->>CG: makeTypeCall('Date')
CG-->>DH: ts.Expression (Type.Date())
DH-->>TH: ts.Expression
TH-->>Out: Emit TypeBox constructs using Type.Date()
note right of Out: Formatting occurs separately (e.g., Prettier)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/handlers/typebox/typebox-type-handlers.ts (1)
1-144: Add @sinclair/typebox as a runtime dependency (>= 0.25.0)
package.json currently has no @sinclair/typebox entry; add, for example,"dependencies": { "@sinclair/typebox": "^0.31.18" }to ensure
Type.Date()support (introduced in v0.25.0) (github.com)
🧹 Nitpick comments (7)
ARCHITECTURE.md (2)
31-31: Tighten wording and call out extended type explicitlyMinor phrasing/grammar and clarity: explicitly reference Type.Date and the extended type system.
-**JavaScript Built-in Types**: Native support for Date type using TypeBox's JavaScript type system +**JavaScript Built-in Types**: Native support for Date via TypeBox’s extended type system (Type.Date())
162-169: Add a compatibility note about non-JSON-Schema extended typesTypeBox’s Date is an extended type. Helpful to set expectations for downstream consumers.
- **Extended Type System**: Leverages TypeBox's JavaScript type system for types that extend beyond standard JSON Schema + - **Extended Type System**: Leverages TypeBox’s extended types (e.g., Type.Date()), which are not representable in plain JSON Schema. If a consumer requires vanilla JSON Schema, consider down-emitting to a string with an appropriate format or documenting the requirement to use TypeBox runtime tooling.src/handlers/typebox/typebox-type-handlers.ts (2)
116-124: Handle qualified type names (e.g., globalThis.Date) in O(1) lookupCurrently only Identifier names are handled. Qualified names like globalThis.Date will miss the fast-path and likely fall through. Support the right-most identifier to keep behavior predictable.
- // O(1) lookup for type references by name - if (!handler && nodeKind === SyntaxKind.TypeReference && Node.isTypeReference(node)) { - const typeNameNode = node.getTypeName() - - if (Node.isIdentifier(typeNameNode)) { - const typeNameText = typeNameNode.getText() - handler = this.typeReferenceHandlers.get(typeNameText) - } - } + // O(1) lookup for type references by name (supports Identifier and QualifiedName) + if (!handler && nodeKind === SyntaxKind.TypeReference && Node.isTypeReference(node)) { + const typeNameNode = node.getTypeName() + let typeNameText: string | undefined + if (Node.isIdentifier(typeNameNode)) { + typeNameText = typeNameNode.getText() + } else if (Node.isQualifiedName(typeNameNode)) { + typeNameText = typeNameNode.getRight().getText() + } + if (typeNameText) { + handler = this.typeReferenceHandlers.get(typeNameText) + } + }If you want, I can add a focused test case asserting support for
type A = globalThis.Date.
81-89: Optional: register other JS built-ins when readyWhen you add BigInt or Symbol support, this is the right map to extend for parity.
src/handlers/typebox/date-type-handler.ts (2)
5-15: Broaden method signatures for LSP compliance and add QualifiedName supportTo stay substitutable with BaseTypeHandler and handle qualified names, widen parameters to Node and narrow internally.
-import { Node, ts, TypeReferenceNode } from 'ts-morph' +import { Node, ts } from 'ts-morph' export class DateTypeHandler extends BaseTypeHandler { - canHandle(node: TypeReferenceNode): boolean { - const typeName = node.getTypeName() - - return Node.isIdentifier(typeName) && typeName.getText() === 'Date' - } + canHandle(node: Node): boolean { + if (!Node.isTypeReference(node)) return false + const typeName = node.getTypeName() + if (Node.isIdentifier(typeName)) return typeName.getText() === 'Date' + if (Node.isQualifiedName(typeName)) return typeName.getRight().getText() === 'Date' + return false + } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - handle(_node: TypeReferenceNode): ts.Expression { + handle(_node: Node): ts.Expression { return makeTypeCall('Date') } }
12-13: Remove unnecessary eslint-disable if underscore-args are ignoredIf your ESLint config ignores args starting with “_”, you can drop this directive. If not, keep as-is.
tests/handlers/typebox/date-types.test.ts (1)
71-81: Add a test for qualified name (globalThis.Date)Covers a common pattern in stricter codebases and ensures the fast-path mapping works.
test('Date in union type', () => { const sourceFile = createSourceFile(project, `type Value = string | Date | number`) @@ }) + + test('qualified Date reference (globalThis.Date)', () => { + const sourceFile = createSourceFile(project, `type A = globalThis.Date`) + + expect(generateFormattedCode(sourceFile)).toBe( + formatWithPrettier(` + export const A = Type.Date(); + + export type A = Static<typeof A>; + `), + ) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
ARCHITECTURE.md(3 hunks)src/handlers/typebox/date-type-handler.ts(1 hunks)src/handlers/typebox/typebox-type-handlers.ts(3 hunks)tests/handlers/typebox/date-types.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/handlers/typebox/date-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/typebox-type-handlers.ts (1)
src/handlers/typebox/date-type-handler.ts (1)
DateTypeHandler(5-16)
tests/handlers/typebox/date-types.test.ts (1)
tests/utils.ts (3)
createSourceFile(12-14)generateFormattedCode(26-37)formatWithPrettier(16-24)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...e using TypeBox's JavaScript type system - Import Resolution: Cross-file type dep...
(QB_NEW_EN)
[grammar] ~166-~166: There might be a mistake here.
Context: ...s them to TypeBox's Type.Date() schema - Type Reference Registration: JavaScrip...
(QB_NEW_EN)
🔇 Additional comments (6)
ARCHITECTURE.md (2)
137-140: Docs update looks correctNew “JavaScript Type Handlers” category and file path read well. No issues.
175-175: Good call-out on O(1) mappingExplicitly mentioning Date in the type reference map is useful for discoverability.
src/handlers/typebox/typebox-type-handlers.ts (3)
6-6: Import registration for Date handler is correctImport location and naming align with the existing convention.
56-56: Instantiation looks goodLocal instance follows the existing handler pattern.
88-88: Registering 'Date' in typeReferenceHandlers is correctDirect O(1) dispatch for Date references is the right place.
tests/handlers/typebox/date-types.test.ts (1)
5-112: Solid coverage across constructsGood breadth: alias, export, object prop, union, array, and function signatures.
This change adds a new
DateTypeHandlerclass that can handle theDatetype in TypeScript. TheDateTypeHandleris registered in theTypeBoxTypeHandlersclass, allowing the codegen to generate TypeBox expressions forDatetypes.This enhancement provides native support for the JavaScript
Datetype, making it easier to work with date-related data in the generated validation schemas.