refactor: extract string keys utility and optimize omit type handler #10
Conversation
This commit introduces two main changes: 1. Adds a new utility function `extractStringKeys` in `key-extraction-utils.ts` to extract string keys from a TypeScript type node. This is useful for handling the key types in `Pick` and `Omit` operations. 2. Optimizes the `OmitTypeHandler` in `omit-type-handler.ts` by using the new `extractStringKeys` utility and the `createTypeBoxKeys` function to generate the TypeBox expression for the omitted keys. This simplifies the code and makes it more readable. The changes improve the overall code quality and maintainability of the validation schema codegen library.
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces a TypeOperatorBaseHandler for TypeScript type operators and refactors KeyOf and Readonly handlers to extend it. Adds node/type utilities and key-extraction helpers. Updates Pick and Omit to use the new key utilities. Adjusts SimpleTypeHandler kind checks. Updates architecture docs to reflect the new structure. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TS as TS AST
participant TH as TypeOperatorBaseHandler
participant U as getTypeBoxType
participant H as Concrete Handler (e.g., KeyOf/Readonly)
participant TB as TypeBox
TS->>TH: Node (TypeOperatorTypeNode)
TH->>TH: canHandle() via isTypeOperatorWithOperator
TH->>TS: node.getTypeNode()
TH->>U: map inner type to TypeBox expr
U-->>TH: innerTypeExpr
TH->>H: createTypeBoxCall(innerTypeExpr)
H-->>TB: makeTypeCall(typeBoxMethod, [innerTypeExpr])
TB-->>TH: TypeBox call expr
TH-->>TS: Result expression
sequenceDiagram
autonumber
actor TS as TS AST
participant PH as Pick/Omit Handler
participant KU as key-extraction-utils
participant U as getTypeBoxType
participant TB as TypeBox
TS->>PH: Node (TypeReference Pick/Omit)
PH->>U: map object type to TypeBox expr
U-->>PH: objectTypeExpr
PH->>KU: extractStringKeys(keysType)
KU-->>PH: [keys...]
PH->>KU: createTypeBoxKeys([keys...])
KU-->>PH: keysExpr
PH->>TB: makeTypeCall('Pick'|'Omit', [objectTypeExpr, keysExpr])
TB-->>PH: Result expression
PH-->>TS: Result expression
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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: 3
🧹 Nitpick comments (11)
src/utils/key-extraction-utils.ts (3)
11-25: Avoid lossy key extraction; detect mixed/non-literal unions and signal to caller.extractStringKeys silently drops non-string-literal members in a union (e.g., 'a' | keyof T), returning a partial subset. That will produce incorrect Pick/Omit semantics if the caller assumes completeness.
Consider returning both the extracted keys and a “complete” flag so callers can reliably fall back to getTypeBoxType(keysType) when extraction isn’t exhaustive.
-export const extractStringKeys = (keysType: Node): string[] => { - const keys: string[] = [] +export const extractStringKeys = (keysType: Node): { keys: string[]; complete: boolean } => { + const keys: string[] = [] + let complete = true if (Node.isUnionTypeNode(keysType)) { for (const unionType of keysType.getTypeNodes()) { if (Node.isLiteralTypeNode(unionType)) { const literalExpression = unionType.getLiteral() if (Node.isStringLiteral(literalExpression)) { keys.push(literalExpression.getLiteralText()) } - } + } else { + complete = false + } } } else if (Node.isLiteralTypeNode(keysType)) { const literalExpression = keysType.getLiteral() if (Node.isStringLiteral(literalExpression)) { keys.push(literalExpression.getLiteralText()) } + } else { + complete = false } - return keys + return { keys, complete } }Follow up: update call sites (Pick/Omit) to use the flag for a safe fallback. I can push the related changes if you want.
27-28: Deterministic output: de-duplicate and sort keys before returning.Union members can contain duplicates via re-exports or type aliases. Ensuring uniqueness and stable order improves cache hits and consistent snapshots.
Apply this small change:
- return keys + const uniqueSorted = Array.from(new Set(keys)).sort() + return uniqueSorted
35-45: Guard against empty key arrays in createTypeBoxKeys.If called with an empty array, this builds Type.Union([]), which is invalid (and differs from intended semantics). Prefer letting the caller decide, but at minimum assert non-empty to catch misuse early.
Two options:
- Preferred: keep this helper pure and require callers to handle empty/partial extraction (see change proposed in omit-type-handler).
- Defensive: add a runtime guard that throws or returns Type.Never().
Example defensive guard:
export const createTypeBoxKeys = (keys: string[]): ts.Expression => { - if (keys.length === 1) { + if (keys.length === 0) { + throw new Error('createTypeBoxKeys: keys must be non-empty') + } + if (keys.length === 1) { return makeTypeCall('Literal', [ts.factory.createStringLiteral(keys[0]!)]) }If you prefer returning Type.Never() instead of throwing, I can adjust.
ARCHITECTURE.md (2)
199-201: Document the fallback contract for key-extraction utilities.You added key-extraction-utils.ts here—great. Please make explicit that:
- extractStringKeys only extracts string-literal members.
- Callers must fall back to getTypeBoxType(keysType) when extraction is empty or incomplete (e.g., unions containing non-literals or keyof).
I can add a short code snippet demonstrating the recommended Pick/Omit pattern with fallback.
212-213: Clarify TypeOperatorBaseHandler usage with node-type-utils.Since TypeOperator handlers rely on operator detection, reference the isTypeOperatorWithOperator helper here and note the expected SyntaxKind (e.g., SyntaxKind.KeyOfKeyword, SyntaxKind.ReadonlyKeyword) to keep implementers aligned.
src/utils/node-type-utils.ts (3)
31-40: Qualified names not handled.isTypeReferenceWithName only matches Identifiers. It won’t catch qualified names like ns.Pick or imported aliases. If that’s intentional, add a short comment; otherwise, consider handling QualifiedName.
- return Node.isIdentifier(typeNameNode) && typeNameNode.getText() === typeName + if (Node.isIdentifier(typeNameNode)) return typeNameNode.getText() === typeName + if (Node.isQualifiedName?.(typeNameNode)) return typeNameNode.getRight().getText() === typeName + return false
45-54: Same qualified-name limitation for any-name variant.Mirror the qualified-name handling here to keep behavior consistent across both helpers.
17-19: Micro: prefer a Set for large kind collections.isAnySyntaxKind does a linear includes; if used on hot paths with >O(1) lookups, consider Set for better average performance. Given the small arrays used today, this is optional.
src/handlers/typebox/keyof-type-handler.ts (1)
9-10: Use the declared typeBoxMethod to avoid string drift.Minor consistency tweak: rely on the class property rather than repeating the literal.
Apply this diff:
- protected createTypeBoxCall(innerType: ts.Expression): ts.Expression { - return makeTypeCall('KeyOf', [innerType]) + protected createTypeBoxCall(innerType: ts.Expression): ts.Expression { + return makeTypeCall(this.typeBoxMethod, [innerType]) }src/handlers/typebox/readonly-type-handler.ts (1)
9-10: Mirror KeyOf: prefer using the typeBoxMethod field.Keeps literals in one place and avoids accidental mismatch.
Apply this diff:
- protected createTypeBoxCall(innerType: ts.Expression): ts.Expression { - return makeTypeCall('Readonly', [innerType]) + protected createTypeBoxCall(innerType: ts.Expression): ts.Expression { + return makeTypeCall(this.typeBoxMethod, [innerType]) }src/handlers/typebox/type-operator-base-handler.ts (1)
11-13: typeBoxMethod is defined but unused in the base; provide a sensible default.You can make createTypeBoxCall non-abstract and implement the common “single-arg passthrough” with this.typeBoxMethod. Concrete handlers can still override when needed (e.g., special argument shapes).
Apply this diff to implement the default method and remove the abstract declaration below:
protected abstract readonly typeBoxMethod: string @@ - protected abstract createTypeBoxCall(innerType: ts.Expression): ts.Expression + protected createTypeBoxCall(innerType: ts.Expression): ts.Expression { + // Default: direct Type.<method>(innerType) + return makeTypeCall(this.typeBoxMethod, [innerType]) + }And add the missing import at the top of this file:
import { makeTypeCall } from '@daxserver/validation-schema-codegen/utils/typebox-codegen-utils'
📜 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 (9)
ARCHITECTURE.md(3 hunks)src/handlers/typebox/keyof-type-handler.ts(1 hunks)src/handlers/typebox/readonly-type-handler.ts(1 hunks)src/handlers/typebox/reference/omit-type-handler.ts(2 hunks)src/handlers/typebox/reference/pick-type-handler.ts(2 hunks)src/handlers/typebox/simple-type-handler.ts(2 hunks)src/handlers/typebox/type-operator-base-handler.ts(1 hunks)src/utils/key-extraction-utils.ts(1 hunks)src/utils/node-type-utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/handlers/typebox/type-operator-base-handler.ts (2)
src/utils/node-type-utils.ts (1)
isTypeOperatorWithOperator(24-26)src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/handlers/typebox/reference/omit-type-handler.ts (1)
src/utils/key-extraction-utils.ts (2)
extractStringKeys(8-28)createTypeBoxKeys(34-45)
src/handlers/typebox/reference/pick-type-handler.ts (1)
src/utils/key-extraction-utils.ts (2)
extractStringKeys(8-28)createTypeBoxKeys(34-45)
src/handlers/typebox/simple-type-handler.ts (2)
src/utils/node-type-utils.ts (1)
isAnySyntaxKind(17-19)src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/utils/key-extraction-utils.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/keyof-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
src/handlers/typebox/readonly-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
🔇 Additional comments (7)
ARCHITECTURE.md (1)
236-240: Verify handler paths and cross-links.Double-check that the documented paths for keyof-type-handler.ts and readonly-type-handler.ts match the actual file locations after the refactor (some repos nest these under a type-operator folder). Update mcfile links accordingly for navigability.
src/utils/node-type-utils.ts (1)
24-26: Node.isTypeOperatorTypeNode is the correct guardAccording to the ts-morph API,
Node.isTypeOperatorTypeNode(node)is indeed the built-in type guard forTypeOperatorTypeNodeand will correctly detect nodes ofSyntaxKind.TypeOperator(jsdocs.io)
No changes are required to this implementation; please disregard the suggestion to replace it.Likely an incorrect or invalid review comment.
src/handlers/typebox/simple-type-handler.ts (2)
8-17: Good refactor toward explicit kind set and guard.Moving to a const tuple + isAnySyntaxKind tightens the contract and improves readability. No issues from my side.
36-38: Indexing safety looks fine.canHandle ensures membership in SimpleKinds, so the assertion cast for indexing kindToTypeBox is safe.
src/handlers/typebox/reference/pick-type-handler.ts (1)
2-5: Good move: centralize key parsing in key-extraction-utils.Importing createTypeBoxKeys and extractStringKeys removes duplicated logic from handlers and improves consistency.
src/handlers/typebox/keyof-type-handler.ts (1)
5-7: Handler alignment with base class looks good.Declaring operatorKind = SyntaxKind.KeyOfKeyword and typeBoxMethod = 'KeyOf' matches the intent. Nice simplification via the base handler.
src/handlers/typebox/readonly-type-handler.ts (1)
5-7: Solid refactor to the operator base.Readonly maps cleanly via operatorKind and typeBoxMethod; keeps concerns nicely separated.
| const typeboxObjectType = getTypeBoxType(objectType) | ||
|
|
||
| let omitKeys: string[] = [] | ||
| if (Node.isUnionTypeNode(keysType)) { | ||
| omitKeys = keysType.getTypeNodes().map((unionType) => { | ||
| if (Node.isLiteralTypeNode(unionType)) { | ||
| const literalExpression = unionType.getLiteral() | ||
| if (Node.isStringLiteral(literalExpression)) { | ||
| return literalExpression.getLiteralText() | ||
| } | ||
| } | ||
| return '' // Should not happen if keys are string literals | ||
| }) | ||
| } else if (Node.isLiteralTypeNode(keysType)) { | ||
| const literalExpression = keysType.getLiteral() | ||
| if (Node.isStringLiteral(literalExpression)) { | ||
| omitKeys = [literalExpression.getLiteralText()] | ||
| } | ||
| } | ||
|
|
||
| let typeboxKeys: ts.Expression | ||
| if (omitKeys.length === 1) { | ||
| typeboxKeys = makeTypeCall('Literal', [ts.factory.createStringLiteral(omitKeys[0]!)]) | ||
| } else { | ||
| typeboxKeys = makeTypeCall('Union', [ | ||
| ts.factory.createArrayLiteralExpression( | ||
| omitKeys.map((k) => makeTypeCall('Literal', [ts.factory.createStringLiteral(k)])), | ||
| true, | ||
| ), | ||
| ]) | ||
| } | ||
| const omitKeys = extractStringKeys(keysType) | ||
| const typeboxKeys = createTypeBoxKeys(omitKeys) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix empty/partial key extraction: avoid Type.Union([]) and lossy omission.
If keysType isn’t a pure union of string literals (e.g., keyof T, or a mixed union), extractStringKeys returns [] or a partial list. Passing that to createTypeBoxKeys builds an invalid union and/or drops members, changing semantics.
Apply a safe fallback to getTypeBoxType(keysType) unless the union is fully string-literal:
- const typeboxObjectType = getTypeBoxType(objectType)
- const omitKeys = extractStringKeys(keysType)
- const typeboxKeys = createTypeBoxKeys(omitKeys)
+ const typeboxObjectType = getTypeBoxType(objectType)
+ const omitKeys = extractStringKeys(keysType)
+ const isAllStringLiteralUnion =
+ (Node.isLiteralTypeNode(keysType) && Node.isStringLiteral(keysType.getLiteral())) ||
+ (Node.isUnionTypeNode(keysType) &&
+ keysType.getTypeNodes().every(
+ (t) => Node.isLiteralTypeNode(t) && Node.isStringLiteral(t.getLiteral()),
+ ))
+ const typeboxKeys =
+ omitKeys.length > 0 && isAllStringLiteralUnion
+ ? createTypeBoxKeys(omitKeys)
+ : getTypeBoxType(keysType)Please mirror the same logic in PickTypeHandler for consistency. I can submit the corresponding change if helpful.
🏁 Script executed:
#!/bin/bash
# Search for extractStringKeys definition and usage
rg -n "extractStringKeys" -C3 --glob '*.ts'
# Search for createTypeBoxKeys definition and usage
rg -n "createTypeBoxKeys" -C3 --glob '*.ts'
# Search for getTypeBoxType definition and usage
rg -n "getTypeBoxType" -C3 --glob '*.ts'
# List TypeBox handler files for context
fd --extension ts src/handlers/typebox/referenceLength of output: 27174
🏁 Script executed:
#!/bin/bash
# Display extractStringKeys and createTypeBoxKeys definitions for verification
sed -n '1,200p' src/utils/key-extraction-utils.tsLength of output: 1546
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/handlers/typebox/reference/pick-type-handler.tsLength of output: 1147
🏁 Script executed:
#!/bin/bash
# Dump first 200 lines of omit-type-handler.ts for review
sed -n '1,200p' src/handlers/typebox/reference/omit-type-handler.tsLength of output: 1147
Prevent invalid key unions in Omit & Pick handlers
Both OmitTypeHandler and PickTypeHandler always call
const keys = extractStringKeys(keysType)
const typeboxKeys = createTypeBoxKeys(keys)but extractStringKeys only returns the full set of string literals when keysType is literally a union of string literals or a single literal. For any other expression—keyof T, T extends U ? …, mixed unions—extractStringKeys returns [] (or a subset), leading to Type.Union([]) or dropped members and a semantic mismatch.
You should guard against this by falling back to the raw keysType via getTypeBoxType(keysType) unless:
keysTypeis a literal or a union of only string-literal type nodes, andextractStringKeysreturned at least one key.
Please apply the same fix in both handlers:
• src/handlers/typebox/reference/omit-type-handler.ts (in handle):
- const typeboxObjectType = getTypeBoxType(objectType)
- const omitKeys = extractStringKeys(keysType)
- const typeboxKeys = createTypeBoxKeys(omitKeys)
+ const typeboxObjectType = getTypeBoxType(objectType)
+ const omitKeys = extractStringKeys(keysType)
+ const isAllStringLiterals =
+ (Node.isLiteralTypeNode(keysType) && Node.isStringLiteral(keysType.getLiteral())) ||
+ (Node.isUnionTypeNode(keysType) &&
+ keysType.getTypeNodes().every(
+ (t) => Node.isLiteralTypeNode(t) && Node.isStringLiteral(t.getLiteral()),
+ ))
+ const typeboxKeys =
+ omitKeys.length > 0 && isAllStringLiterals
+ ? createTypeBoxKeys(omitKeys)
+ : getTypeBoxType(keysType)• src/handlers/typebox/reference/pick-type-handler.ts (in handle):
- const typeboxObjectType = getTypeBoxType(objectType)
- const pickKeys = extractStringKeys(keysType)
- const typeboxKeys = createTypeBoxKeys(pickKeys)
+ const typeboxObjectType = getTypeBoxType(objectType)
+ const pickKeys = extractStringKeys(keysType)
+ const isAllStringLiterals =
+ (Node.isLiteralTypeNode(keysType) && Node.isStringLiteral(keysType.getLiteral())) ||
+ (Node.isUnionTypeNode(keysType) &&
+ keysType.getTypeNodes().every(
+ (t) => Node.isLiteralTypeNode(t) && Node.isStringLiteral(t.getLiteral()),
+ ))
+ const typeboxKeys =
+ pickKeys.length > 0 && isAllStringLiterals
+ ? createTypeBoxKeys(pickKeys)
+ : getTypeBoxType(keysType)This ensures that whenever we can’t reliably extract every literal key, we let TypeBox handle the original keysType expression instead of generating an empty or lossy union.
🤖 Prompt for AI Agents
In src/handlers/typebox/reference/omit-type-handler.ts around lines 22-25, and
likewise in src/handlers/typebox/reference/pick-type-handler.ts in the
equivalent spot, currently you call extractStringKeys(keysType) and then
unconditionally createTypeBoxKeys(omitKeys). Change this to: call
extractStringKeys(keysType) as before, but only call createTypeBoxKeys(omitKeys)
when keysType is a string-literal or a union of string-literals AND
extractStringKeys returned a non-empty array; otherwise call
getTypeBoxType(keysType) and use that as the TypeBox representation of the keys.
Implement the literal-or-union check (e.g. helper
isStringLiteralOrUnion(keysType) or inline AST checks) and use it to choose
between createTypeBoxKeys(omitKeys) and getTypeBoxType(keysType) so we fall back
to the original keysType when extraction is unreliable.
| const pickKeys = extractStringKeys(keysType) | ||
| const typeboxKeys = createTypeBoxKeys(pickKeys) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Bug risk: empty/unsupported keys produce Type.Union([]); add a safe fallback and dedupe.
When keysType is not a union of string literal types (e.g., Pick<T, keyof T>, numeric/symbol keys, aliased unions), extractStringKeys returns [], and createTypeBoxKeys([]) builds Type.Union([]). That’s likely invalid for Type.Pick and deviates from TS semantics (Pick<T, never> -> {}), causing incorrect codegen. Use getTypeBoxType(keysType) as a fallback, and dedupe keys to avoid repeated literals.
Apply this diff:
- const pickKeys = extractStringKeys(keysType)
- const typeboxKeys = createTypeBoxKeys(pickKeys)
+ const pickKeys = extractStringKeys(keysType)
+ const typeboxKeys =
+ pickKeys.length > 0
+ ? createTypeBoxKeys([...new Set(pickKeys)])
+ : getTypeBoxType(keysType)Follow-up:
- Consider explicitly handling the “no keys” case by emitting a keys expression that results in an empty object schema (mirroring Pick<T, never>). If TypeBox lacks a canonical representation for “never keys,” the above fallback is still safer than emitting an empty union.
Run quick checks to find potentially affected call sites:
🏁 Script executed:
#!/usr/bin/env bash
# Pick/Omit with `keyof` (unsupported by extractStringKeys) that may regress.
rg -nP --type=ts -C2 '\bPick<[^,]+,\s*keyof\b'
rg -nP --type=ts -C2 '\bOmit<[^,]+,\s*keyof\b'
# Picks with non-string literal unions
rg -nP --type=ts -C2 '\bPick<[^,]+,\s*[^\'"]' | rg -nP -v "['\"]"Length of output: 214
🏁 Script executed:
#!/usr/bin/env bash
# Quick check: Pick/Omit with `keyof` (unsupported by extractStringKeys)
rg -nP --type=ts -C2 "\\bPick<[^,]+,\\s*keyof\\b"
rg -nP --type=ts -C2 "\\bOmit<[^,]+,\\s*keyof\\b"
# Note: picks with non-string-literal unions are more complex to detect reliably via regex;
# please review manually for cases like Pick<T, MyUnionOfKeys> where MyUnionOfKeys isn’t a string literal union.Length of output: 97
Enforce safe fallback and dedupe for Pick-type key handling
A search for direct Pick<…, keyof …> and Omit<…, keyof …> patterns in the codebase returned no hits, so there are no immediate regressions. Nevertheless, to prevent emitting Type.Union([]) when extractStringKeys yields an empty array—and to mirror the TypeScript semantics of Pick<T, never> → {}—please apply this change in src/handlers/typebox/reference/pick-type-handler.ts (around lines 23–25):
- const pickKeys = extractStringKeys(keysType)
- const typeboxKeys = createTypeBoxKeys(pickKeys)
+ const pickKeys = extractStringKeys(keysType)
+ const typeboxKeys =
+ pickKeys.length > 0
+ ? createTypeBoxKeys([...new Set(pickKeys)])
+ : getTypeBoxType(keysType)• Dedupe literal keys via new Set(pickKeys).
• Fallback to getTypeBoxType(keysType) to safely handle empty or unsupported unions.
📝 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 pickKeys = extractStringKeys(keysType) | |
| const typeboxKeys = createTypeBoxKeys(pickKeys) | |
| const pickKeys = extractStringKeys(keysType) | |
| const typeboxKeys = | |
| pickKeys.length > 0 | |
| ? createTypeBoxKeys([...new Set(pickKeys)]) | |
| : getTypeBoxType(keysType) |
🤖 Prompt for AI Agents
In src/handlers/typebox/reference/pick-type-handler.ts around lines 23 to 25,
the current logic calls extractStringKeys(keysType) then
createTypeBoxKeys(pickKeys) which can produce Type.Union([]) when pickKeys is
empty; dedupe the keys by using new Set(pickKeys) before passing to
createTypeBoxKeys, and if the resulting array is empty, return
getTypeBoxType(keysType) as a safe fallback (this prevents emitting an empty
union and mirrors TypeScript's Pick<T, never> → {}). Ensure the code transforms
the Set back to an array for createTypeBoxKeys and only calls createTypeBoxKeys
when that array is non-empty; otherwise call and return
getTypeBoxType(keysType).
| handle(node: TypeOperatorTypeNode): ts.Expression { | ||
| const innerType = node.getTypeNode() | ||
| const typeboxInnerType = getTypeBoxType(innerType) | ||
| return this.createTypeBoxCall(typeboxInnerType) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm override compatibility of handle(...) with BaseTypeHandler.
This class overrides handle(node: TypeOperatorTypeNode), which is narrower than a common BaseTypeHandler signature of handle(node: Node). If BaseTypeHandler indeed declares handle(node: Node), this override will not be type-compatible in TS. If so, change the signature to Node and narrow inside.
Run:
If BaseTypeHandler expects Node, consider:
- handle(node: TypeOperatorTypeNode): ts.Expression {
- const innerType = node.getTypeNode()
+ handle(node: Node): ts.Expression {
+ // canHandle() guarantees TypeOperatorTypeNode; assert to satisfy typing
+ const opNode = node as TypeOperatorTypeNode
+ const innerType = opNode.getTypeNode()
const typeboxInnerType = getTypeBoxType(innerType)
return this.createTypeBoxCall(typeboxInnerType)
}🏁 Script executed:
#!/usr/bin/env bash
# Inspect BaseTypeHandler.handle signature and all overrides.
rg -nP --type=ts -C3 'class\s+BaseTypeHandler\b'
rg -nP --type=ts -C2 'abstract\s+handle\s*\(\s*node\s*:\s*Node'
rg -nP --type=ts -C2 'handle\s*\(\s*node\s*:\s*TypeOperatorTypeNode'Length of output: 1772
Ensure handle signature matches BaseTypeHandler
The base class declares
abstract handle(node: Node): ts.Expressionbut here you’ve overridden it with a narrower parameter type (TypeOperatorTypeNode), which breaks compatibility (and will error if you use the override keyword or enable stricter checks). Please update the signature to accept Node and then narrow internally:
- handle(node: TypeOperatorTypeNode): ts.Expression {
- const innerType = node.getTypeNode()
+ handle(node: Node): ts.Expression {
+ // `canHandle` already ensures this is a TypeOperatorTypeNode
+ const opNode = node as TypeOperatorTypeNode
+ const innerType = opNode.getTypeNode()
const typeboxInnerType = getTypeBoxType(innerType)
return this.createTypeBoxCall(typeboxInnerType)
}• File needing update:
– src/handlers/typebox/type-operator-base-handler.ts (around line 18)
🤖 Prompt for AI Agents
In src/handlers/typebox/type-operator-base-handler.ts around lines 18 to 22, the
handle method signature currently uses a narrower TypeOperatorTypeNode which
mismatches BaseTypeHandler's abstract handle(node: Node): ts.Expression; change
the method signature to handle(node: Node): ts.Expression and inside the method
perform a runtime narrowing (e.g. assert/if check or cast) to obtain a
TypeOperatorTypeNode before using getTypeNode/getTypeBoxType/createTypeBoxCall
so the override is compatible with the base class and strict checks.
This commit introduces two main changes:
Adds a new utility function
extractStringKeysinkey-extraction-utils.tstoextract string keys from a TypeScript type node. This is useful for handling
the key types in
PickandOmitoperations.Optimizes the
OmitTypeHandlerinomit-type-handler.tsby using the newextractStringKeysutility and thecreateTypeBoxKeysfunction to generatethe TypeBox expression for the omitted keys. This simplifies the code and
makes it more readable.
The changes improve the overall code quality and maintainability of the
validation schema codegen library.