feat(typebox): improve handling of large union types #24
Conversation
This commit introduces a new feature to handle large union types more efficiently in the TypeBox schema codegen. The key changes are: - Introduce a `shouldChunkUnion` function to check if a union type has more than a certain number of members (20 in this case). - Implement the `createChunkNodes` function to split large union types into smaller "chunk" types, each containing a subset of the original union members. - Update the `addLocalTypes` function in `local-type-collector.ts` to detect large unions and create the corresponding chunk types. - The chunk types are added to the `NodeGraph` and `ResolverStore` to be processed by the schema codegen. This change improves the performance and memory usage of the schema codegen when dealing with large union types, which can be common in complex validation schemas.
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces TypeBoxContext across handlers and utilities, replaces makeTypeCall with GenericTypeUtils, adds chunked-union support (chunk nodes, ChunkParser, printer wiring), threads NodeGraph through traversal → printer → parsers, updates getTypeBoxType to require context, adjusts parsers to use GenericTypeUtils and context, removes legacy helpers, and adds integration tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant G as generateCode
participant DT as DependencyTraversal
participant P as TypeBoxPrinter
participant CP as ChunkParser
participant GU as GenericTypeUtils
participant TBU as getTypeBoxType
G->>DT: traverse()
G->>P: printSortedNodes(traversal, printer)
Note over P: nodeGraph = traversal.getNodeGraph()
loop For each traversed node
P->>P: detect 'chunk' or typeAlias with chunkReferences
alt type == 'chunk' OR has chunkReferences
P->>CP: parseChunk(traversedNode, aliasName)
CP->>TBU: getTypeBoxType(node, { nodeGraph })
TBU-->>CP: ts.Expression (TypeBox)
CP->>GU: addTypeBoxVariableStatement(..., isExported: false/true)
CP->>GU: addStaticTypeAlias(...)
else Other node types
P->>Parsers: parse with context { nodeGraph }
end
end
sequenceDiagram
autonumber
participant RH as RecordTypeHandler
participant TBU as getTypeBoxType
participant GU as GenericTypeUtils
participant NG as NodeGraph
RH->>NG: resolve key type reference
alt key type has chunkReferences
RH->>NG: validate referenced chunk nodes
loop each chunk
RH->>TBU: getTypeBoxType(valueType, ctx)
TBU-->>RH: valueExpr
RH->>GU: makeTypeCall('Record', [Identifier(chunk), valueExpr])
end
RH->>GU: makeTypeCall('Intersect', [ArrayLiteral(records)])
else no chunks
RH->>TBU: getTypeBoxType(keyType, ctx)
RH->>TBU: getTypeBoxType(valueType, ctx)
RH->>GU: makeTypeCall('Record', [keyExpr, valueExpr])
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/handlers/typebox/simple-type-handler.ts (1)
31-38: handle signature must include TypeBoxContext (mismatch with BaseTypeHandler).
Current override omits context; this will fail to compile after the abstract signature change.Apply:
+import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call' @@ - handle(node: Node): ts.Expression { + handle(node: Node, _context: TypeBoxContext): ts.Expression { return GenericTypeUtils.makeTypeCall(kindToTypeBox[node.getKind() as SimpleKind]) }src/handlers/typebox/template-literal-type-handler.ts (1)
11-12: Update handle signature to include context.
Must match BaseTypeHandler.Apply:
+import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call' @@ - handle(node: TemplateLiteralTypeNode): ts.Expression { + handle(node: TemplateLiteralTypeNode, _context: TypeBoxContext): ts.Expression {src/handlers/typebox/type-query-handler.ts (1)
21-27: QualifiedName replacement only replaces first dot; can emit invalid identifiersfullName.replace('.', '_') misses nested qualifiers (e.g., a.b.c). Replace all dots.
- const fullName = exprName.getText() - return ts.factory.createIdentifier(fullName.replace('.', '_')) + const fullName = exprName.getText() + return ts.factory.createIdentifier(fullName.replace(/\./g, '_'))src/handlers/typebox/object/interface-type-handler.ts (1)
59-74: Bug: extended generic types can emit invalid identifiers (breaks nested/multi-arg generics).
When parseGenericTypeCall fails (e.g., Foo<Bar> or Map<K, V>), fallback uses ts.factory.createIdentifier(typeText). Identifiers cannot contain '<', '>', ',', or dots, yielding invalid code and wrong schemas.Apply this minimal fix to resolve extended types via the central pipeline:
@@ -import { HeritageClause, InterfaceDeclaration, Node, ts } from 'ts-morph' +import { HeritageClause, InterfaceDeclaration, Node, ts } from 'ts-morph' +import { getTypeBoxType } from '@daxserver/validation-schema-codegen/utils/typebox-call' @@ - handle(node: InterfaceDeclaration, context: TypeBoxContext): ts.Expression { + handle(node: InterfaceDeclaration, context: TypeBoxContext): ts.Expression { @@ - const extendedTypes = this.collectExtendedTypes(heritageClauses) + const extendedTypes = this.collectExtendedTypes(heritageClauses, context) @@ - private collectExtendedTypes(heritageClauses: HeritageClause[]): ts.Expression[] { + private collectExtendedTypes( + heritageClauses: HeritageClause[], + context: TypeBoxContext, + ): ts.Expression[] { @@ - for (const typeNode of heritageClause.getTypeNodes()) { - const typeText = typeNode.getText() - extendedTypes.push( - this.parseGenericTypeCall(typeText) ?? ts.factory.createIdentifier(typeText), - ) - } + for (const typeNode of heritageClause.getTypeNodes()) { + extendedTypes.push(getTypeBoxType(typeNode, context)) + }Follow-up: remove parseGenericTypeCall/createTypeExpression as dead code once this change lands.
src/handlers/typebox/object/object-like-base-handler.ts (2)
57-60: Numeric property keys emitted as identifiers (invalid).
123cannot be an identifier. Return a NumericLiteral to preserve{ 123: ... }.Apply this diff:
- } else if (Node.isNumericLiteral(nameNode)) { - // Numeric properties can be used as identifiers - propName = nameNode.getLiteralValue().toString() - shouldUseIdentifier = true + } else if (Node.isNumericLiteral(nameNode)) { + // Preserve numeric literal property names (e.g., { 1: ... }) + return ts.factory.createNumericLiteral(nameNode.getLiteralValue().toString())Also applies to: 66-69
53-56: Add reserved word validation to identifier checks
TheisValidIdentifierutility only enforces character rules viats.isIdentifierStart/isIdentifierPartand does not filter out JavaScript reserved keywords (e.g.default,class,function), which would currently pass and break unquoted key handling inobject-like-base-handler.ts(lines 53–56). Introduce a reserved-words check (e.g. against a Set of ECMAScript/TypeScript keywords) before accepting an unquoted property name.
🧹 Nitpick comments (30)
tsconfig.build.json (1)
16-17: Consider trimming redundant excludes for simplicity.With include restricted to "src//*", excluding "tests//*", "dist", and "node_modules" is redundant. Keep only test globs if you want to guard future include changes.
"include": ["src/**/*"], - "exclude": ["tests/**/*", "**/*.test.ts", "**/*.spec.ts", "dist", "node_modules"] + "exclude": ["**/*.test.ts", "**/*.spec.ts"]src/traverse/dependency-traversal.ts (1)
71-74: NodeGraph usage is read-only and safe Consumers (TypeBoxPrinter, GraphVisualizer) only read the graph, andgetNodeGraph()is only called afterstartTraversal(). No external mutation detected. Optional: expose asReadonly<NodeGraph>or add a doc comment to signal immutability.src/traverse/types.ts (1)
5-13: Prefer discriminated union over duplicate isChunk flag; make chunkReferences required for chunk nodes.
type: 'chunk'already discriminates;isChunkis redundant and invites drift. Also,chunkReferencesshould be required whentype === 'chunk'for stronger typing.Illustrative shape:
type NodeKind = 'interface' | 'typeAlias' | 'enum' | 'function' | 'chunk' type TraversedNode = Base & ( | { type: 'chunk'; chunkReferences: string[] } | { type: Exclude<NodeKind, 'chunk'> } )If keeping the interface, at least deprecate
isChunkand documentchunkReferencessemantics (e.g., qualified names of chunk members).src/parsers/base-parser.ts (1)
1-22: Add runtime guard and JSDoc for nodeGraph in BaseParserAll parser subclasses and instantiations already forward
nodeGraph, so adding a defensive check and documenting the option improves future safety:
- In
src/parsers/base-parser.ts, prepend a JSDoc onBaseParserOptions.nodeGraph:/** * Graph of AST nodes to traverse. * @readonly */ nodeGraph: NodeGraph- Update the constructor to validate
nodeGraph:constructor(options: BaseParserOptions) { this.newSourceFile = options.newSourceFile this.printer = options.printer this.processedTypes= options.processedTypes - this.nodeGraph = options.nodeGraph + if (!options.nodeGraph) throw new Error('BaseParser requires nodeGraph') + this.nodeGraph = options.nodeGraph }src/utils/generic-type-utils.ts (1)
151-169: DefaultcompilerNodetonewSourceFile.compilerNodeand reorder parameters
Change the signature to:static addStaticTypeAlias( newSourceFile: SourceFile, name: string, printer: ts.Printer, compilerNode: ts.SourceFile = newSourceFile.compilerNode, ) { … }Then update all call sites from:
GenericTypeUtils.addStaticTypeAlias( this.newSourceFile, typeName, this.compilerNode, this.printer, )to:
GenericTypeUtils.addStaticTypeAlias( this.newSourceFile, typeName, this.printer, ) // (add a fourth argument only when you need a non‐default compilerNode)Affected files:
- src/parsers/parse-type-aliases.ts
- src/parsers/parse-interfaces.ts
- src/parsers/parse-chunks.ts (2 occurrences)
- src/parsers/parse-function-declarations.ts
- src/parsers/parse-enums.ts
- docs/utilities.md
src/handlers/typebox/simple-type-handler.ts (1)
6-6: Unused constant.
TypeBoxType isn’t referenced; consider removing to avoid drift.src/handlers/typebox/date-type-handler.ts (1)
6-10: Optional: support qualified names for Date.
If users reference globalThis.Date or a qualified Date, broaden the check.src/index.ts (1)
101-103: Optionally decouple printSortedNodes from traversalConsider passing NodeGraph directly instead of the whole DependencyTraversal to keep printSortedNodes focused on printing concerns.
-import type { SourceFile } from 'ts-morph' +import type { SourceFile } from 'ts-morph' +import type { NodeGraph } from '@daxserver/validation-schema-codegen/traverse/node-graph' -const printSortedNodes = ( - sortedTraversedNodes: TraversedNode[], - newSourceFile: SourceFile, - dependencyTraversal: DependencyTraversal, -) => { +const printSortedNodes = ( + sortedTraversedNodes: TraversedNode[], + newSourceFile: SourceFile, + nodeGraph: NodeGraph, +) => { const printer = new TypeBoxPrinter({ newSourceFile, printer: ts.createPrinter(), - nodeGraph: dependencyTraversal.getNodeGraph(), + nodeGraph, }) } -const result = printSortedNodes(traversedNodes, newSourceFile, dependencyTraversal) +const result = printSortedNodes(traversedNodes, newSourceFile, dependencyTraversal.getNodeGraph())src/parsers/parse-enums.ts (1)
22-31: Use GenericTypeUtils.addTypeBoxVariableStatement for consistency (optional)Minor consistency win and less boilerplate.
Apply:
- this.newSourceFile.addVariableStatement({ - isExported: true, - declarationKind: VariableDeclarationKind.Const, - declarations: [ - { - name: schemaName, - initializer: typeboxType, - }, - ], - }) + GenericTypeUtils.addTypeBoxVariableStatement(this.newSourceFile, schemaName, typeboxType, true)If you adopt this, VariableDeclarationKind can be dropped from the imports.
src/handlers/typebox/indexed-access-type-handler.ts (2)
50-55: Handle const-array initializers for typeof A[number]Variable declarations often lack an explicit type node (const A = ['a','b'] as const), so extract from the initializer to preserve literal unions.
Apply:
const variableDeclaration = sourceFile.getVariableDeclaration(typeName) if (variableDeclaration) { - const tupleUnion = this.extractTupleUnion(variableDeclaration.getTypeNode()) + const tupleUnion = + this.extractTupleUnion(variableDeclaration.getTypeNode()) ?? + this.extractTupleUnionFromInitializer(variableDeclaration.getInitializer()) if (tupleUnion) return tupleUnion }Add this helper (outside the selected range):
private extractTupleUnionFromInitializer(expr: Node | undefined): ts.Expression | null { if (!expr) return null // Unwrap `as const` if (expr.isKind(ts.SyntaxKind.AsExpression)) { expr = expr.asKindOrThrow(ts.SyntaxKind.AsExpression).getExpression() } const arr = expr.asKind?.(ts.SyntaxKind.ArrayLiteralExpression) if (!arr) return null const literals: ts.Expression[] = [] for (const el of arr.getElements()) { const str = el.asKind?.(ts.SyntaxKind.StringLiteral) if (str) { literals.push(GenericTypeUtils.makeTypeCall('Literal', [ts.factory.createStringLiteral(str.getLiteralValue())])) } } if (literals.length === 0) return null // Consider integrating union chunking here (see comment above). return GenericTypeUtils.makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literals)]) }
39-56: Resolve identifiers beyond same-file scope (optional)Current lookup only checks same-file type aliases/variables and ignores QualifiedName/imported/namespace cases. Using the type checker to resolve the symbol (getSymbol → getDeclarations) would improve correctness for typeof ns.A[number] and imported const tuples.
src/handlers/typebox/object/interface-type-handler.ts (1)
44-57: Optional: remove ad-hoc generic parsing and primitive mapping.
After adopting getTypeBoxType for extends, parseGenericTypeCall/createTypeExpression become redundant and brittle (single type arg only). Safe to delete to reduce surface area.- private parseGenericTypeCall(typeText: string): ts.Expression | null { ... } - - private createTypeExpression(typeArg: string): ts.Expression { ... }src/handlers/typebox/collection/collection-base-handler.ts (1)
14-16: Formatting/readability for large unions.
Emit multi-line arrays to keep huge unions readable in generated code.- const arrayLiteral = ts.factory.createArrayLiteralExpression(typeBoxTypes) + const arrayLiteral = ts.factory.createArrayLiteralExpression(typeBoxTypes, true)src/parsers/parse-type-aliases.ts (2)
26-29: Tiny cleanup: reuse a context const.Avoids re-allocating the same object literal repeatedly.
- const typeboxTypeNode = typeNode - ? getTypeBoxType(typeNode, { nodeGraph: this.nodeGraph }) - : GenericTypeUtils.makeTypeCall('Any') + const ctx = { nodeGraph: this.nodeGraph } + const typeboxTypeNode = typeNode + ? getTypeBoxType(typeNode, ctx) + : GenericTypeUtils.makeTypeCall('Any')
49-52: Deduplicate fallback logic with a helper (optional).Same Any-fallback pattern appears twice; consider a small helper to keep this DRY.
src/handlers/typebox/template-literal-type-processor.ts (1)
49-65: Broaden union literal handling by delegating to processType for each member.Current logic only preserves string literals in unions; number/boolean literals downgrade to
Type.String(). Prefer mapping each member throughprocessTypeto retain numeric/boolean literal fidelity.Apply:
- private static processUnionType(unionType: ts.UnionTypeNode): ts.Expression { - const unionParts = unionType.types.map((t) => { - if (t.kind === ts.SyntaxKind.LiteralType) { - const literalType = t as ts.LiteralTypeNode - if (ts.isStringLiteral(literalType.literal)) { - return GenericTypeUtils.makeTypeCall('Literal', [ - ts.factory.createStringLiteral(literalType.literal.text), - ]) - } - } - return GenericTypeUtils.makeTypeCall('String') // fallback - }) - - return GenericTypeUtils.makeTypeCall('Union', [ - ts.factory.createArrayLiteralExpression(unionParts), - ]) - } + private static processUnionType(unionType: ts.UnionTypeNode): ts.Expression { + const unionParts = unionType.types.map((t) => this.processType(t)) + return GenericTypeUtils.makeTypeCall('Union', [ + ts.factory.createArrayLiteralExpression(unionParts), + ]) + }src/utils/typebox-call.ts (1)
12-20: All getTypeBoxType calls include a context argument; no missing contexts detected. Optional: add a dev-only guard to throw when context is absent and avoid silent fallback:export const getTypeBoxType = (node?: Node, context?: TypeBoxContext): ts.Expression => { - if (!node || !context) return GenericTypeUtils.makeTypeCall('Any') + if (!node || !context) { + if (process.env.NODE_ENV !== 'production') { + throw new Error('getTypeBoxType: context is required') + } + return GenericTypeUtils.makeTypeCall('Any') + }src/traverse/local-type-collector.ts (3)
8-13: Threshold semantics: use “> CHUNK_SIZE” to match “exceeding a threshold.”
>= CHUNK_SIZEwill chunk exactly-20-member unions into a single chunk, adding overhead with no benefit and potentially producingType.Union([OnlyOneChunk]).- return typeNode.getTypeNodes().length >= CHUNK_SIZE + return typeNode.getTypeNodes().length > CHUNK_SIZE
37-76: Optional: avoid accumulating many temp files or reuse a scratch file.If this runs over many unions, virtual temp files will grow unbounded. Consider:
- building the union TypeNode via ts.factory (no temp file), or
- using a single per-source-file scratch with
overwrite: true.
39-46: Name collision risk across modules.Chunk variable names are plain
${parentTypeName}_Chunk${i+1}. If two files define the same parent type, collisions can occur in the single generated output file.Consider suffixing with a short hash of
sourceFile.getFilePath()when derivingchunkName(e.g.,_h123abcde).Also applies to: 69-76
src/handlers/typebox/function-type-handler.ts (1)
17-27: Clarify handling for rest parametersRest params aren’t represented; emitting them as ordinary params is incorrect. Either support them explicitly or fail fast with a clear error.
Do you want me to add a guard (e.g., throw with a helpful message) or prototype
Type.Rest(...)support if your TypeBox version provides it?src/handlers/typebox/reference/record-type-handler.ts (1)
52-56: Minor: compute value type once
getTypeBoxType(valueType, context)needn’t run per chunk; the diff above hoists it. Good for performance and clarity.src/handlers/typebox/reference/omit-type-handler.ts (1)
17-28: Consider chunked key unions for Omit<> as wellWith the fallback to
getTypeBoxType(keysType, context), chunked key unions will now flow through correctly. Please add a test coveringOmit<T, KeysChunked>to prevent regressions.I can add an integration test mirroring the
Recordchunked-key path if helpful.src/handlers/typebox/object/object-like-base-handler.ts (1)
21-29: Extract Optional-call detection into a small util.Centralize “is Type.Optional(...)” detection to avoid copy/paste and handle edge cases (e.g., parenthesized expressions) consistently.
tests/integration/deep-instantiation-integration.test.ts (3)
5-11: Add threshold boundary test (exactly 20 members shouldn’t chunk).Cover the no-chunk-at-threshold case to lock behavior.
I can add a test “should not chunk at exactly 20”.
47-63: Add an integration test forPickwith aliased keys.Ensures the new fallback avoids
Type.Union([])and supportstype Keys = ...; type C = Pick<A, Keys>.I can draft the test case.
306-360: Consider asserting chunk name stability.Add expectations ensuring chunk order/naming (
A_Chunk1..n) is deterministic to prevent flaky diffs.src/handlers/typebox/keyof-typeof-handler.ts (1)
75-89: Large keyof unions may bypass the new chunking path.If objects can have > threshold keys, consider chunking here too (using the passed context) to keep memory usage predictable.
Would keyof targets ever exceed the union chunk threshold in your datasets? If yes, I can sketch a context-aware path that emits chunk aliases instead of a monolithic union literal array.
src/parsers/parse-chunks.ts (2)
71-82: Determinism and deduping for chunk references.Guard against duplicates and ensure stable order for reproducible output.
- const chunkIdentifiers = traversedNode.chunkReferences.map((qualifiedName) => { + const refs = Array.from(new Set(traversedNode.chunkReferences)).sort() + const chunkIdentifiers = refs.map((qualifiedName) => { const chunkNode = this.nodeGraph.getNode(qualifiedName) if (!chunkNode) { throw new Error(`Chunk node not found: ${qualifiedName}`) } // Use the original name of the chunk as an identifier return ts.factory.createIdentifier(chunkNode.originalName) })
10-14: Redundant re-declarations in ChunkParserOptions.newSourceFile, printer, processedTypes already exist on BaseParserOptions. Remove to reduce noise.
📜 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 (45)
src/handlers/typebox/base-type-handler.ts(1 hunks)src/handlers/typebox/collection/array-type-handler.ts(1 hunks)src/handlers/typebox/collection/collection-base-handler.ts(1 hunks)src/handlers/typebox/collection/intersection-type-handler.ts(1 hunks)src/handlers/typebox/collection/tuple-type-handler.ts(1 hunks)src/handlers/typebox/collection/union-type-handler.ts(1 hunks)src/handlers/typebox/date-type-handler.ts(2 hunks)src/handlers/typebox/function-type-handler.ts(1 hunks)src/handlers/typebox/indexed-access-type-handler.ts(4 hunks)src/handlers/typebox/keyof-typeof-handler.ts(3 hunks)src/handlers/typebox/literal-type-handler.ts(2 hunks)src/handlers/typebox/object/interface-type-handler.ts(3 hunks)src/handlers/typebox/object/object-like-base-handler.ts(2 hunks)src/handlers/typebox/object/object-type-handler.ts(1 hunks)src/handlers/typebox/reference/omit-type-handler.ts(1 hunks)src/handlers/typebox/reference/partial-type-handler.ts(1 hunks)src/handlers/typebox/reference/pick-type-handler.ts(1 hunks)src/handlers/typebox/reference/readonly-type-handler.ts(1 hunks)src/handlers/typebox/reference/record-type-handler.ts(1 hunks)src/handlers/typebox/reference/required-type-handler.ts(1 hunks)src/handlers/typebox/simple-type-handler.ts(2 hunks)src/handlers/typebox/template-literal-type-handler.ts(3 hunks)src/handlers/typebox/template-literal-type-processor.ts(4 hunks)src/handlers/typebox/type-query-handler.ts(2 hunks)src/handlers/typebox/type-reference-handler.ts(3 hunks)src/handlers/typebox/type/type-operator-base-handler.ts(2 hunks)src/handlers/typebox/typeof-type-handler.ts(2 hunks)src/index.ts(2 hunks)src/parsers/base-parser.ts(1 hunks)src/parsers/parse-chunks.ts(1 hunks)src/parsers/parse-enums.ts(2 hunks)src/parsers/parse-function-declarations.ts(4 hunks)src/parsers/parse-interfaces.ts(2 hunks)src/parsers/parse-type-aliases.ts(2 hunks)src/printer/typebox-printer.ts(1 hunks)src/traverse/dependency-traversal.ts(1 hunks)src/traverse/local-type-collector.ts(2 hunks)src/traverse/types.ts(1 hunks)src/utils/add-static-type-alias.ts(0 hunks)src/utils/generic-type-utils.ts(2 hunks)src/utils/key-extraction-utils.ts(2 hunks)src/utils/typebox-call.ts(1 hunks)src/utils/typebox-codegen-utils.ts(0 hunks)tests/integration/deep-instantiation-integration.test.ts(1 hunks)tsconfig.build.json(1 hunks)
💤 Files with no reviewable changes (2)
- src/utils/add-static-type-alias.ts
- src/utils/typebox-codegen-utils.ts
🧰 Additional context used
🧬 Code graph analysis (40)
src/traverse/dependency-traversal.ts (1)
src/traverse/node-graph.ts (1)
NodeGraph(8-38)
src/handlers/typebox/date-type-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/type-query-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/collection/union-type-handler.ts (1)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)
src/handlers/typebox/object/interface-type-handler.ts (2)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/object/object-type-handler.ts (1)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)
src/handlers/typebox/template-literal-type-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/utils/typebox-call.ts (3)
src/handlers/typebox/typebox-type-handlers.ts (1)
TypeBoxTypeHandlers(28-145)src/traverse/node-graph.ts (1)
NodeGraph(8-38)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/template-literal-type-processor.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/parsers/parse-type-aliases.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
tests/integration/deep-instantiation-integration.test.ts (1)
tests/utils.ts (3)
createSourceFile(12-14)generateFormattedCode(26-37)formatWithPrettier(16-24)
src/traverse/local-type-collector.ts (3)
src/traverse/node-graph.ts (1)
NodeGraph(8-38)src/utils/resolver-store.ts (1)
resolverStore(174-174)src/traverse/types.ts (1)
TraversedNode(3-14)
src/handlers/typebox/reference/omit-type-handler.ts (3)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)src/utils/key-extraction-utils.ts (2)
extractStringKeys(8-28)createTypeBoxKeys(34-46)
src/handlers/typebox/type/type-operator-base-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/utils/key-extraction-utils.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/parsers/base-parser.ts (1)
src/traverse/node-graph.ts (1)
NodeGraph(8-38)
src/handlers/typebox/reference/record-type-handler.ts (3)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/resolver-store.ts (1)
resolverStore(174-174)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/literal-type-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/index.ts (2)
src/traverse/dependency-traversal.ts (1)
DependencyTraversal(15-75)src/printer/typebox-printer.ts (1)
TypeBoxPrinter(17-87)
src/handlers/typebox/collection/intersection-type-handler.ts (1)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)
src/handlers/typebox/keyof-typeof-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/collection/collection-base-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/reference/required-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/reference/readonly-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/reference/pick-type-handler.ts (3)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)src/utils/key-extraction-utils.ts (2)
extractStringKeys(8-28)createTypeBoxKeys(34-46)
src/handlers/typebox/simple-type-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/indexed-access-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/collection/array-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/base-type-handler.ts (1)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)
src/parsers/parse-chunks.ts (4)
src/parsers/base-parser.ts (1)
BaseParserOptions(4-9)src/traverse/types.ts (1)
TraversedNode(3-14)src/utils/typebox-call.ts (1)
getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/function-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/collection/tuple-type-handler.ts (1)
src/utils/typebox-call.ts (1)
TypeBoxContext(8-10)
src/parsers/parse-enums.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/type-reference-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/typeof-type-handler.ts (1)
src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/object/object-like-base-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/printer/typebox-printer.ts (3)
src/traverse/node-graph.ts (1)
NodeGraph(8-38)src/parsers/parse-chunks.ts (1)
ChunkParser(16-97)src/traverse/types.ts (1)
TraversedNode(3-14)
src/parsers/parse-interfaces.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/handlers/typebox/reference/partial-type-handler.ts (2)
src/utils/typebox-call.ts (2)
TypeBoxContext(8-10)getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
src/parsers/parse-function-declarations.ts (2)
src/utils/typebox-call.ts (1)
getTypeBoxType(12-20)src/utils/generic-type-utils.ts (1)
GenericTypeUtils(6-181)
🔇 Additional comments (38)
tsconfig.build.json (1)
16-17: No-op formatting change; config stays equivalent. LGTM.Arrays reflowed to single-line without behavioral impact. Build semantics remain unchanged.
src/utils/generic-type-utils.ts (2)
14-18: Export-control switch is a welcome addition.The
isExportedflag makes this helper more generally usable.
171-180: Allow optional type arguments in makeTypeCallSmall extension enables emitting calls like
Type.Parameters<Foo<T>>()if needed later.- static makeTypeCall(method: string, args: ts.Expression[] = []) { + static makeTypeCall(method: string, args: ts.Expression[] = [], typeArgs?: ts.TypeNode[]) { return ts.factory.createCallExpression( ts.factory.createPropertyAccessExpression( ts.factory.createIdentifier('Type'), ts.factory.createIdentifier(method), ), - undefined, + typeArgs, args, ) }Verified no existing call site passes a third argument—this change is safe and compiles unchanged.
src/utils/key-extraction-utils.ts (2)
1-1: Refactor to GenericTypeUtils is consistent.
34-46: Confirm empty-keys fallback semantic
The revisedcreateTypeBoxKeysnow returnsType.Never()for an emptykeysarray. Verify thatNeveris the intended substitute forType.Union([]). If you’d rather model “no keys” as an empty object (Type.Object({})),undefined(Type.Undefined()), or throw during codegen, please update accordingly.src/handlers/typebox/simple-type-handler.ts (1)
2-2: Migration to GenericTypeUtils looks good.
Centralizing Type calls via GenericTypeUtils is consistent with the PR direction.src/handlers/typebox/date-type-handler.ts (1)
2-2: Swapping to GenericTypeUtils is fine.
Keeps TypeBox call construction uniform.src/handlers/typebox/collection/union-type-handler.ts (1)
10-13: Context-aware union handling LGTM.
Forwarding (types, 'Union', context) into processTypeCollection aligns with the new flow.src/handlers/typebox/template-literal-type-handler.ts (2)
3-4: GenericTypeUtils migration is consistent and correct.
Literal/TemplateLiteral construction looks good and preserves behavior.Also applies to: 19-21, 37-39, 45-46, 49-51
30-33: Verify whether TemplateLiteralTypeProcessor now requires context.
If the processor was updated to be context-aware, pass it through.If needed, apply:
- const processedType = TemplateLiteralTypeProcessor.processType(compilerNode.type) + const processedType = TemplateLiteralTypeProcessor.processType(compilerNode.type, _context)src/index.ts (1)
48-57: Good wiring: printer now receives NodeGraphPassing dependencyTraversal.getNodeGraph() into TypeBoxPrinter is correct and aligns with chunk parsing needs.
src/handlers/typebox/object/object-type-handler.ts (1)
2-3: Context threading looks correctType-only import and passing context into processProperties are consistent with the new API.
Also applies to: 10-13
src/handlers/typebox/collection/array-type-handler.ts (1)
2-4: LGTM on context-aware array handlingUses getTypeBoxType with context and routes via GenericTypeUtils. Matches the new conventions.
Also applies to: 12-16
src/parsers/parse-enums.ts (1)
33-38: Switch to GenericTypeUtils.addStaticTypeAlias is correctPrinter-aware alias generation aligns with the refactor and keeps Static formation centralized.
src/handlers/typebox/type/type-operator-base-handler.ts (1)
20-25: Context-aware operator handling LGTMPropagating context into getTypeBoxType and using GenericTypeUtils.makeTypeCall keeps this in line with the new API.
src/handlers/typebox/collection/intersection-type-handler.ts (1)
10-11: Context forwarded correctly to collection processorMatches the collection-base signature; nothing else to change here.
src/handlers/typebox/object/interface-type-handler.ts (3)
2-3: LGTM: switched to GenericTypeUtils and context import.
26-27: LGTM: Composite constructed via GenericTypeUtils.
11-16: Approve: processProperties correctly accepts and propagates the context parameter. ObjectLikeBaseHandler.processProperties signature includes an optional context param and both interface-type-handler.ts and object-type-handler.ts invoke it with context as intended.src/handlers/typebox/collection/collection-base-handler.ts (1)
8-17: LGTM: context-aware collection processing and GenericTypeUtils usage.src/handlers/typebox/type-reference-handler.ts (1)
13-36: LGTM: context threading and arg conversion via getTypeBoxType.src/handlers/typebox/reference/readonly-type-handler.ts (1)
13-18: Context-aware refactor looks correct.Signature + context threading and the GenericTypeUtils call are consistent with the new handler contract.
src/handlers/typebox/reference/partial-type-handler.ts (1)
13-20: Good migration to context + shared TypeBox call utility.Inner type resolution via getTypeBoxType(innerType, context) and wrapping with Type.Partial is correct.
src/handlers/typebox/reference/required-type-handler.ts (1)
11-16: LGTM on Required handler migration.Contract matches the refactor and uses GenericTypeUtils consistently.
src/handlers/typebox/typeof-type-handler.ts (1)
2-3: Migration to GenericTypeUtils is consistent.Import replacement is fine and matches repo-wide changes.
src/parsers/parse-type-aliases.ts (1)
36-41: Static alias emission path looks good.Using GenericTypeUtils.addStaticTypeAlias keeps this consistent with other parsers.
src/printer/typebox-printer.ts (1)
11-21: Wiring NodeGraph through printer/parsers looks correct.The additions to PrinterOptions, field initialization, and parserOptions propagation are coherent and consistent with the new ChunkParser needs.
Also applies to: 31-39, 45-46
src/parsers/parse-interfaces.ts (1)
25-39: LGTM on context-aware getTypeBoxType and GenericTypeUtils usage.Passing
{ nodeGraph: this.nodeGraph }and centralizing via GenericTypeUtils aligns with the broader migration. Please ensure generated files importTSchemaandStaticwhere needed.Also applies to: 45-73
src/handlers/typebox/reference/record-type-handler.ts (1)
37-41: No changes needed for chunk identifier fallback
The fallback to chunkQualifiedName is never executed—every chunkReference has a corresponding node with a valid originalName—so chunkIdentifier is always safe.Likely an incorrect or invalid review comment.
src/handlers/typebox/object/object-like-base-handler.ts (2)
40-41: LGTM: switched toGenericTypeUtils.makeTypeCall('Object', ...).Consistent with the utilities migration.
9-13: Requirecontextparameter inprocessProperties
No call sites omit the second argument, so makingcontextrequired won’t break existing code.--- a/src/handlers/typebox/object/object-like-base-handler.ts +++ b/src/handlers/typebox/object/object-like-base-handler.ts @@ -9,7 +9,7 @@ export abstract class ObjectLikeBaseHandler extends BaseTypeHandler { protected processProperties( properties: PropertySignature[], - context?: TypeBoxContext, + context: TypeBoxContext, ): ts.PropertyAssignment[] { const propertyAssignments: ts.PropertyAssignment[] = []src/handlers/typebox/reference/pick-type-handler.ts (1)
17-18: LGTM: handler now threadsTypeBoxContextand usesGenericTypeUtils.Aligns with the broader context-aware migration.
src/parsers/parse-function-declarations.ts (2)
23-26: Context correctly threaded into getTypeBoxType (params and return).Migration to GenericTypeUtils.makeTypeCall('Any') for fallbacks is consistent and safe.
Also applies to: 44-47
75-80: Static alias via GenericTypeUtils.addStaticTypeAlias — LGTM.Keeps alias generation centralized with the new utilities.
src/handlers/typebox/keyof-typeof-handler.ts (2)
16-16: Reasonable “Any” fallbacks.Defensive defaults on invalid nodes are fine.
Also applies to: 19-19, 22-22
83-89: Utility migration (Literal/Union) is correct.Construction via GenericTypeUtils.makeTypeCall is consistent with the new API.
src/parsers/parse-chunks.ts (2)
35-48: Ensure declaration order to avoid TDZ at module init.Main union references chunk consts; if emitted before chunk consts, runtime will throw (TDZ). Verify the printer/traversal guarantees chunks are emitted first, or insert chunk consts before the main union explicitly.
I can add an insertion index or a two-pass emit (chunks → main) if ordering isn’t guaranteed.
Also applies to: 89-96
44-47: Non-exported chunk vars — LGTM.Keeps internal chunks out of the public surface while allowing local reference.
| import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call' | ||
| import { Node, ts } from 'ts-morph' | ||
|
|
||
| export abstract class BaseTypeHandler { | ||
| abstract canHandle(node: Node): boolean | ||
| abstract handle(node: Node): ts.Expression | ||
| abstract handle(node: Node, context: TypeBoxContext): ts.Expression | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Abstract signature update looks correct; this is a breaking change—verify all subclasses.
Good use of a type-only import. Please ensure all handlers now implement (node, context).
Run to find handlers missing a context parameter:
🏁 Script executed:
#!/bin/bash
# Handlers extending BaseTypeHandler
rg -nP --type ts -C2 'class\s+\w+\s+extends\s+BaseTypeHandler\b' src
# handle() overrides that don't include a `context:` parameter
rg -nP --type ts 'handle\s*\((?![^)]*context\s*:)' src/handlers/typeboxLength of output: 7830
Ensure all handlers implement the updated handle(node, context) signature
The following subclasses of BaseTypeHandler still use the old handle(...) signature and must be updated to handle(node: X, context: TypeBoxContext): ts.Expression:
- src/handlers/typebox/typeof-type-handler.ts (line 10)
- src/handlers/typebox/type-query-handler.ts (line 10)
- src/handlers/typebox/template-literal-type-handler.ts (line 11)
- src/handlers/typebox/literal-type-handler.ts (line 10)
- src/handlers/typebox/keyof-typeof-handler.ts (line 14)
- src/handlers/typebox/simple-type-handler.ts (line 36)
- src/handlers/typebox/date-type-handler.ts (line 12)
- src/handlers/typebox/type-operator-handler.ts (line 11)
🤖 Prompt for AI Agents
In src/handlers/typebox/base-type-handler.ts lines 1-7 and the listed
subclasses, the abstract handle signature was changed to handle(node: Node,
context: TypeBoxContext): ts.Expression but several subclasses still use the old
signature; update each subclass file
(src/handlers/typebox/typeof-type-handler.ts line 10, type-query-handler.ts line
10, template-literal-type-handler.ts line 11, literal-type-handler.ts line 10,
keyof-typeof-handler.ts line 14, simple-type-handler.ts line 36,
date-type-handler.ts line 12, type-operator-handler.ts line 11) so their class
method signature is handle(node: <specific Node type>, context: TypeBoxContext):
ts.Expression, import TypeBoxContext where needed, adjust any usages inside the
methods to use the new context parameter, and ensure the return type is
ts.Expression; also update any callers to pass the context if they currently
omit it.
| protected processTypeCollection( | ||
| nodes: Node[], | ||
| typeBoxFunction: string, | ||
| context: TypeBoxContext, | ||
| ): ts.Expression { | ||
| const typeBoxTypes = nodes.map((node) => getTypeBoxType(node, context)) | ||
| const arrayLiteral = ts.factory.createArrayLiteralExpression(typeBoxTypes) | ||
|
|
||
| return makeTypeCall(typeBoxFunction, [arrayLiteral]) | ||
| return GenericTypeUtils.makeTypeCall(typeBoxFunction, [arrayLiteral]) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against empty collections (defensive).
If nodes is empty, Type.Union([])/Type.Intersect([]) may not be meaningful. Consider returning Type.Never/Type.Object({}) per your conventions, or assert upstream.
🏁 Script executed:
#!/bin/bash
# List all call sites to ensure `nodes.length > 0` is enforced upstream
rg -nP --type=ts -C2 '\bprocessTypeCollection\s*\(' src/handlersLength of output: 1954
Guard empty collections in processTypeCollection. Add at the top of processTypeCollection (in collection-base-handler.ts) a check for nodes.length === 0 that returns your agreed fallback instead of calling, for example, Type.Union([]) or Type.Intersect([]). For instance, return Type.Never() for unions/intersections or Type.Object({}) for empty tuples per project conventions.
🤖 Prompt for AI Agents
In src/handlers/typebox/collection/collection-base-handler.ts around lines 8 to
17, add a guard at the top of processTypeCollection to return the project
fallback when nodes.length === 0 instead of calling the TypeBox collection
functions with an empty array; specifically, detect empty nodes and return
Type.Never() for union/intersect cases or Type.Object({}) for empty tuple cases
(use the existing GenericTypeUtils.makeTypeCall or appropriate helper to
construct these fallback TypeBox calls), otherwise continue to build the
arrayLiteral and call the original typeBoxFunction.
| handle(node: TupleTypeNode, context: TypeBoxContext): ts.Expression { | ||
| const elements = node.getElements() | ||
| return this.processTypeCollection(elements, 'Tuple', context) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Context-plumbed Tuple handler looks correct; verify all call sites pass context
The signature and forwarding into processTypeCollection are consistent with the new context-aware API.
Run to catch any remaining handlers/call-sites missing the context param:
🏁 Script executed:
#!/bin/bash
# 1) Handlers without a `context: TypeBoxContext` param
rg -nP --type=ts '\bhandle\s*\(\s*[^)]*\)' src/handlers/typebox | rg -nPv 'context\s*:\s*TypeBoxContext' || true
# 2) processTypeCollection calls lacking the context arg
rg -nP --type=ts 'processTypeCollection\([^,]+,\s*["'\''](Tuple|Array|Intersect|Union)["'\'']\s*\)' src/handlers/typebox || trueLength of output: 995
Add missing context parameter to handlers
The following handle() methods lack the required context: TypeBoxContext parameter and need to forward it to their processing calls:
- src/handlers/typebox/typeof-type-handler.ts:10
- src/handlers/typebox/type-query-handler.ts:10
- src/handlers/typebox/template-literal-type-handler.ts:11
- src/handlers/typebox/simple-type-handler.ts:36
- src/handlers/typebox/literal-type-handler.ts:10
- src/handlers/typebox/keyof-typeof-handler.ts:14
- src/handlers/typebox/date-type-handler.ts:12
- src/handlers/typebox/type-operator-handler.ts:11
Update each signature to handle(..., context: TypeBoxContext) and pass context into the underlying process* calls.
🤖 Prompt for AI Agents
In src/handlers/typebox/collection/tuple-type-handler.ts around lines 10 to 12,
the handle method is missing the required context parameter and does not forward
it to the processing call; change the signature to include context:
TypeBoxContext and pass that context into this.processTypeCollection(elements,
'Tuple', context) so the handler matches the other TypeBox handlers and forwards
context to downstream processing.
| handle(): ts.Expression { | ||
| return makeTypeCall('Date') | ||
| return GenericTypeUtils.makeTypeCall('Date') | ||
| } |
There was a problem hiding this comment.
handle signature must accept (node, context).
It currently takes no params and will not satisfy BaseTypeHandler.
Apply:
+import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call'
@@
- handle(): ts.Expression {
+ handle(_node: TypeReferenceNode, _context: TypeBoxContext): ts.Expression {
return GenericTypeUtils.makeTypeCall('Date')
}📝 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.
| handle(): ts.Expression { | |
| return makeTypeCall('Date') | |
| return GenericTypeUtils.makeTypeCall('Date') | |
| } | |
| // at the top of src/handlers/typebox/date-type-handler.ts | |
| import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call' | |
| // …within your handler class… | |
| handle(_node: TypeReferenceNode, _context: TypeBoxContext): ts.Expression { | |
| return GenericTypeUtils.makeTypeCall('Date') | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/date-type-handler.ts around lines 12 to 14, the handle
method signature currently has no parameters but must match BaseTypeHandler by
accepting (node, context); change the method signature to handle(node, context)
and update any references inside to use the provided parameters (or keep them
unused but present), update imports/types if necessary so the types for node and
context are correct, and ensure the file compiles and the class implements the
BaseTypeHandler interface with the new signature.
|
|
||
| handle(node: FunctionTypeNode): ts.Expression { | ||
| handle(node: FunctionTypeNode, context: TypeBoxContext): ts.Expression { | ||
| const parameters = node.getParameters() |
There was a problem hiding this comment.
Exclude this parameter from runtime schema generation
this parameters are type-only and must not appear in Type.Function’s parameter list.
Apply this diff:
- const parameters = node.getParameters()
+ const parameters = node.getParameters().filter((p) => !p.isThisParameter())📝 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.
| const parameters = node.getParameters() | |
| const parameters = node.getParameters().filter((p) => !p.isThisParameter()) |
🤖 Prompt for AI Agents
In src/handlers/typebox/function-type-handler.ts around line 13, the code
collects parameters with node.getParameters() but must exclude the type-only
`this` parameter from the runtime schema; filter the parameters list to remove
any parameter that is the `this` parameter (e.g., using
parameter.isThisParameter() or checking the parameter name === 'this') before
generating the Type.Function parameter list so `this` does not appear in the
emitted TypeBox schema.
| handle(): ts.Expression { | ||
| // TypeQuery represents 'typeof' expressions in TypeScript | ||
| // For TypeBox, we'll return a String type as typeof returns string literals | ||
| return makeTypeCall('String') | ||
| return GenericTypeUtils.makeTypeCall('String') | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bug: handle() signature mismatches the handler contract and logic is incorrect for TypeQuery.
- getTypeBoxType calls handlers via handle(node, context); this handler exposes handle() with no params → compile/runtime break.
- Returning Type.String for a TypeQuery is semantically wrong (TypeScript’s typeof type query does not yield “string literals”).
Proposed minimal fix (align signature and avoid wrong typing until proper support is implemented):
-import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler'
-import { GenericTypeUtils } from '@daxserver/validation-schema-codegen/utils/generic-type-utils'
+import { BaseTypeHandler } from '@daxserver/validation-schema-codegen/handlers/typebox/base-type-handler'
+import { GenericTypeUtils } from '@daxserver/validation-schema-codegen/utils/generic-type-utils'
+import type { TypeBoxContext } from '@daxserver/validation-schema-codegen/utils/typebox-call'
export class TypeofTypeHandler extends BaseTypeHandler {
canHandle(node: Node): boolean {
return Node.isTypeQuery(node)
}
- handle(): ts.Expression {
- // TypeQuery represents 'typeof' expressions in TypeScript
- // For TypeBox, we'll return a String type as typeof returns string literals
- return GenericTypeUtils.makeTypeCall('String')
+ handle(_node: Node, _context: TypeBoxContext): ts.Expression {
+ // TODO: Implement proper TypeQuery handling by resolving the referenced symbol’s type.
+ // Fallback to Any to avoid misrepresenting the type.
+ return GenericTypeUtils.makeTypeCall('Any')
}
}Run to confirm no other handlers still use the old handle() shape:
🏁 Script executed:
#!/bin/bash
rg -nP -C1 'class\s+\w+TypeHandler\b' src/handlers | sed -n 'p'
rg -nP 'handle\s*\(' src/handlers | sed -n 'p'Length of output: 9016
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'class\s+\w+TypeHandler\b' src/handlers/typebox/type-query-handler.ts
rg -nP -C3 'handle' src/handlers/typebox/type-query-handler.ts
rg -nP -C3 'class\s+\w+TypeHandler\b' src/handlers/typebox/date-type-handler.ts
rg -nP -C3 'handle' src/handlers/typebox/date-type-handler.ts
rg -nP -C3 'class\s+\w+TypeHandler\b' src/handlers/typebox/typeof-type-handler.ts
rg -nP -C3 'handle' src/handlers/typebox/typeof-type-handler.tsLength of output: 2593
Refactor TypeBox handlers: align handle signatures and correct fallback types
- In
src/handlers/typebox/typeof-type-handler.ts, change
handle(): ts.Expression➔handle(_node: Node, _context: TypeBoxContext): ts.Expression,
importtype { TypeBoxContext }and update the return toGenericTypeUtils.makeTypeCall('Any'). - In
src/handlers/typebox/date-type-handler.ts, change
handle(): ts.Expression➔handle(_node: TypeReferenceNode, _context: TypeBoxContext): ts.Expressionand importTypeBoxContext. - In
src/handlers/typebox/type-query-handler.ts, change
handle(node: TypeQueryNode): ts.Expression➔handle(_node: TypeQueryNode, _context: TypeBoxContext): ts.Expression, importTypeBoxContext, and returnGenericTypeUtils.makeTypeCall('Any')as a fallback until proper resolution is implemented.
🤖 Prompt for AI Agents
In src/handlers/typebox/typeof-type-handler.ts around lines 10 to 14, update the
handler signature to match the refactor by changing handle(): ts.Expression to
handle(_node: Node, _context: TypeBoxContext): ts.Expression, add an import for
type { TypeBoxContext } from the appropriate module, and change the fallback
return from GenericTypeUtils.makeTypeCall('String') to
GenericTypeUtils.makeTypeCall('Any').
| if (!traversedNode.chunkReferences || traversedNode.chunkReferences.length === 0) { | ||
| // Fallback to empty union if no chunk references | ||
| const unionCall = GenericTypeUtils.makeTypeCall('Union', []) | ||
| const typeboxType = this.printer.printNode( | ||
| ts.EmitHint.Expression, | ||
| unionCall, | ||
| this.newSourceFile.compilerNode, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Bug: calling Type.Union() with no arguments.
Type.Union expects an array; an empty call is invalid. For “no chunk refs,” emit Type.Never() (or Type.Union([]) if you explicitly want an empty union).
- const unionCall = GenericTypeUtils.makeTypeCall('Union', [])
+ const unionCall = GenericTypeUtils.makeTypeCall('Never')📝 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.
| if (!traversedNode.chunkReferences || traversedNode.chunkReferences.length === 0) { | |
| // Fallback to empty union if no chunk references | |
| const unionCall = GenericTypeUtils.makeTypeCall('Union', []) | |
| const typeboxType = this.printer.printNode( | |
| ts.EmitHint.Expression, | |
| unionCall, | |
| this.newSourceFile.compilerNode, | |
| ) | |
| if (!traversedNode.chunkReferences || traversedNode.chunkReferences.length === 0) { | |
| // Fallback to empty union if no chunk references | |
| const unionCall = GenericTypeUtils.makeTypeCall('Never') | |
| const typeboxType = this.printer.printNode( | |
| ts.EmitHint.Expression, | |
| unionCall, | |
| this.newSourceFile.compilerNode, | |
| ) |
🤖 Prompt for AI Agents
In src/parsers/parse-chunks.ts around lines 51 to 59, the code calls
GenericTypeUtils.makeTypeCall('Union', []) when traversedNode.chunkReferences is
empty which results in an invalid zero-argument Union call; change this to emit
a Type.Never() instead (i.e., call GenericTypeUtils.makeTypeCall('Never', [])
and print that), so the no-chunk case produces a valid Type.Never() node
(alternatively, if you deliberately want an empty union, ensure Union is called
with an explicit empty-array AST representation, but prefer Never here).
| const { node, aliasName, type } = traversedNode | ||
|
|
||
| // Handle chunk nodes first | ||
| if (type === 'chunk') { | ||
| this.chunkParser.parseChunk(traversedNode, aliasName) | ||
| return | ||
| } | ||
|
|
||
| // Handle type aliases with chunk references using chunk parser | ||
| if ( | ||
| type === 'typeAlias' && | ||
| traversedNode.chunkReferences && | ||
| traversedNode.chunkReferences.length > 0 | ||
| ) { | ||
| this.chunkParser.parseChunk(traversedNode, aliasName) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure chunk consts are emitted before main unions to avoid TDZ at runtime.
Type.Union([Chunk1, ...]) will reference chunk variables; if those consts are printed after the main alias, you risk a temporal-dead-zone ReferenceError. Please either:
- guarantee traversal order (e.g., add graph dependencies from main alias → chunk nodes and topologically sort), or
- have
parseChunkfor the main alias eagerly emit referenced chunks before emitting the union.
See my suggested dependency wiring in local-type-collector.ts.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the typebox-printer.ts file
FILE=$(fd --full-path typebox-printer.ts | head -n1)
echo "Found file: $FILE"
# Display lines 1–200 for context
sed -n '1,200p' "$FILE"Length of output: 3231
🏁 Script executed:
#!/bin/bash
set -e
# Find the parse-chunks implementation file
FILE=$(fd --full-path parse-chunks.ts | head -n1 || echo "")
echo "Parse-chunks file: $FILE"
# If found, show class and method definitions
if [ -f "$FILE" ]; then
grep -R "class ChunkParser" -n "$FILE"
grep -R "parseChunk" -n "$FILE"
sed -n '1,200p' "$FILE"
fiLength of output: 4215
Ensure chunk constants are declared before unions that reference them
In the typeAlias-with-chunks branch you call parseChunk(traversedNode) which emits a Union([…chunkIdentifiers]) before ever emitting the individual chunk const declarations—this will trigger a TDZ ReferenceError at runtime. Either:
- In
ChunkParser.parseChunk, iterate overtraversedNode.chunkReferencesand recursively callparseChunkon each before emitting the union, or - Adjust your traversal to topologically sort nodes so all chunk-type nodes are printed before any main union aliases.
🤖 Prompt for AI Agents
In src/printer/typebox-printer.ts around lines 49 to 66, the typeAlias branch
calls this.chunkParser.parseChunk(traversedNode, aliasName) which currently
emits a Union([...chunkIdentifiers]) before the individual chunk const
declarations, causing TDZ ReferenceErrors; update the implementation so chunk
declarations are produced before any unions that reference them — either modify
ChunkParser.parseChunk to first iterate over traversedNode.chunkReferences and
recursively call parseChunk for each referenced chunk (guarding against
cycles/duplication) before emitting the union, or change the traversal here to
topologically sort nodes so all chunk-type nodes are printed first; ensure
deduplication/visited checks to avoid infinite recursion.
| const project = node.getProject() | ||
| const tempSourceFile = project.createSourceFile( | ||
| `__temp_chunk_${i}.ts`, | ||
| `type TempChunk = ${chunkTypeNodes.map((node) => node.getText()).join(' | ')}`, | ||
| ) | ||
| const tempTypeAlias = tempSourceFile.getTypeAliases()[0]! |
There was a problem hiding this comment.
Bug: temporary source file name collisions across chunked types.
project.createSourceFile("__temp_chunk_${i}.ts") will throw on the second union processed (same filename) unless overwrite: true is set. Also, filenames should be unique per (file, type, chunk).
Apply:
- const tempSourceFile = project.createSourceFile(
- `__temp_chunk_${i}.ts`,
- `type TempChunk = ${chunkTypeNodes.map((node) => node.getText()).join(' | ')}`,
- )
+ const fileStem = sourceFile.getBaseNameWithoutExtension()
+ const safeParent = parentTypeName.replace(/[^A-Za-z0-9_]/g, '_')
+ const tempFilePath = `__typebox_chunks/${fileStem}__${safeParent}__chunk_${i}.ts`
+ const tempSourceFile = project.createSourceFile(
+ tempFilePath,
+ `type TempChunk = ${chunkTypeNodes.map((n) => n.getText()).join(' | ')}`,
+ { overwrite: true },
+ )Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/traverse/local-type-collector.ts around lines 50 to 55, the temporary
source file name "__temp_chunk_${i}.ts" can collide across chunks and subsequent
createSourceFile calls will throw; make the filename unique per original
file/type/chunk and allow overwriting. Construct a deterministic unique name
(e.g. include the original source file path or basename, the parent type name or
id, and the chunk index) and call project.createSourceFile(uniqueFilename,
content, { overwrite: true }) so repeated runs or different chunks won’t error.
| const chunkReferences = createChunkNodes( | ||
| typeNode, | ||
| typeName, | ||
| nodeGraph, | ||
| maincodeNodeIds, | ||
| requiredNodeIds, | ||
| sourceFile, | ||
| ) | ||
|
|
||
| // Add the main type as a regular type alias that will be handled by chunk parser | ||
| nodeGraph.addTypeNode(qualifiedName, { | ||
| node: typeAlias, | ||
| type: 'typeAlias', | ||
| originalName: typeName, | ||
| qualifiedName, | ||
| isImported: false, | ||
| isMainCode: true, | ||
| chunkReferences: chunkReferences, | ||
| }) | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add graph dependencies from main type → chunk nodes to enforce print order.
This guarantees chunks are emitted before the main union and avoids TDZ at runtime, regardless of set/map iteration order.
const chunkReferences = createChunkNodes(
typeNode,
typeName,
nodeGraph,
maincodeNodeIds,
requiredNodeIds,
sourceFile,
)
// Add the main type as a regular type alias that will be handled by chunk parser
nodeGraph.addTypeNode(qualifiedName, {
node: typeAlias,
type: 'typeAlias',
originalName: typeName,
qualifiedName,
isImported: false,
isMainCode: true,
chunkReferences: chunkReferences,
})
+ // Ensure chunks are processed before the main union
+ for (const chunkQn of chunkReferences) {
+ nodeGraph.addDependency(qualifiedName, chunkQn)
+ }📝 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.
| const chunkReferences = createChunkNodes( | |
| typeNode, | |
| typeName, | |
| nodeGraph, | |
| maincodeNodeIds, | |
| requiredNodeIds, | |
| sourceFile, | |
| ) | |
| // Add the main type as a regular type alias that will be handled by chunk parser | |
| nodeGraph.addTypeNode(qualifiedName, { | |
| node: typeAlias, | |
| type: 'typeAlias', | |
| originalName: typeName, | |
| qualifiedName, | |
| isImported: false, | |
| isMainCode: true, | |
| chunkReferences: chunkReferences, | |
| }) | |
| } else { | |
| const chunkReferences = createChunkNodes( | |
| typeNode, | |
| typeName, | |
| nodeGraph, | |
| maincodeNodeIds, | |
| requiredNodeIds, | |
| sourceFile, | |
| ) | |
| // Add the main type as a regular type alias that will be handled by chunk parser | |
| nodeGraph.addTypeNode(qualifiedName, { | |
| node: typeAlias, | |
| type: 'typeAlias', | |
| originalName: typeName, | |
| qualifiedName, | |
| isImported: false, | |
| isMainCode: true, | |
| chunkReferences: chunkReferences, | |
| }) | |
| // Ensure chunks are processed before the main union | |
| for (const chunkQn of chunkReferences) { | |
| nodeGraph.addDependency(qualifiedName, chunkQn) | |
| } | |
| } else { |
🤖 Prompt for AI Agents
In src/traverse/local-type-collector.ts around lines 126 to 145, after adding
the main type node to nodeGraph, add explicit graph dependencies from the main
type (qualifiedName) to each chunk node returned in chunkReferences so chunks
are emitted first; iterate chunkReferences and for each chunk id call the
graph-edge API (e.g. nodeGraph.addEdge(qualifiedName, chunkId) or
nodeGraph.addDependency(qualifiedName, chunkId) depending on the existing API)
to create edges main→chunk, ensuring print order prevents TDZ.
This commit introduces a new feature to handle large union types more efficiently in the TypeBox schema codegen.
The key changes are:
shouldChunkUnionfunction to check if a union type has more than a certain number of members (20 in this case).createChunkNodesfunction to split large union types into smaller "chunk" types, each containing a subset of the original union members.addLocalTypesfunction inlocal-type-collector.tsto detect large unions and create the corresponding chunk types.NodeGraphandResolverStoreto be processed by the schema codegen.This change improves the performance and memory usage of the schema codegen when dealing with large union types, which can be common in complex validation schemas.