feat(typebox): add support for keyof typeof expressions #18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The changes in this commit add support for handling `keyof typeof` expressions in the TypeBox type handler system. This is an important feature as `keyof typeof` is a common TypeScript construct used to extract the keys of an object type. The key changes are: - Added a new `KeyOfTypeofHandler` class that extends the `BaseTypeHandler` and handles `keyof typeof` expressions. - Integrated the `KeyOfTypeofHandler` into the overall handler system, ensuring it is prioritized over the regular `KeyOfTypeHandler`. - Added test cases to cover various scenarios for `keyof typeof` handling, including single string, single numeric, multiple string, multiple numeric, and mixed properties. These changes will allow the TypeBox type handler to correctly generate the appropriate TypeBox types for `keyof typeof` expressions, improving the overall functionality and robustness of the validation schema codegen tool.
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new TypeBox handler for keyof typeof expressions, wires it into the handler pipeline with higher precedence, updates documentation to reflect the new handler and supported types, and introduces tests validating code generation for various key sets. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CG as Codegen
participant TH as TypeBoxTypeHandlers
participant KTT as KeyOfTypeofHandler
participant KTH as KeyOfTypeHandler
participant AST as TS AST/Source
CG->>TH: request type for node
TH->>KTT: canHandle(node)?
alt node is "keyof typeof Identifier"
KTT->>AST: resolve Identifier declaration
alt extract keys succeeds
KTT-->>TH: Type.Union/Literal of keys
else no keys/invalid
KTT-->>TH: Type.Any
end
else not applicable
TH->>KTH: canHandle(node)?
KTH-->>TH: handle or pass
end
TH-->>CG: TypeBox expression
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 5
🧹 Nitpick comments (6)
docs/overview.md (1)
14-14: Code-format TS keywords for consistency.Render TS syntax as code to match the rest of the docs.
- - keyof typeof expressions + - `keyof typeof` expressionsdocs/handler-system.md (1)
45-46: Clarify handler precedence in docs.Call out that KeyOfTypeofHandler is evaluated before KeyofTypeHandler to avoid confusion.
-- `KeyofTypeHandler` - keyof T -- `KeyOfTypeofHandler` - keyof typeof obj +- `KeyofTypeHandler` - keyof T +- `KeyOfTypeofHandler` - keyof typeof obj (evaluated before `KeyofTypeHandler`)src/handlers/typebox/keyof-typeof-handler.ts (2)
38-45: Unwrap common wrappers (as,satisfies, parens) before probing initializer.This improves robustness for patterns like
const A = ({ a: 1 }) as constor usingsatisfies.- // Try object literal - let initializer = varDecl.getInitializer() - if (Node.isAsExpression(initializer)) { - initializer = initializer.getExpression() - } + // Try object literal + let initializer = varDecl.getInitializer() + while ( + initializer && + (Node.isAsExpression(initializer) || + Node.isSatisfiesExpression?.(initializer) || + Node.isParenthesizedExpression(initializer)) + ) { + initializer = initializer.getExpression() + }
18-21: Support qualified names (typeof ns.A).
getExprName()can be a qualified name. Consider acceptingQualifiedNameand using the rightmost identifier or symbol resolution to locate declarations (including imports).tests/handlers/typebox/keyof-typeof.test.ts (2)
32-32: Fix typo in test name.- test('shoud handle single numeric', () => { + test('should handle single numeric', () => {
123-123: Add a test for empty objects (should beNever).This guards the
keyof {}edge case.+ test('should handle empty object as Never', () => { + const sourceFile = createSourceFile( + project, + ` + const A = {}; + type T = keyof typeof A + `, + ) + + expect(generateFormattedCode(sourceFile)).toBe( + formatWithPrettier(` + export const T = Type.Never(); + + export type T = Static<typeof T>; + `), + ) + })
📜 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 (5)
docs/handler-system.md(1 hunks)docs/overview.md(1 hunks)src/handlers/typebox/keyof-typeof-handler.ts(1 hunks)src/handlers/typebox/typebox-type-handlers.ts(3 hunks)tests/handlers/typebox/keyof-typeof.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/handlers/typebox/keyof-typeof.test.ts (1)
tests/utils.ts (3)
createSourceFile(12-14)generateFormattedCode(26-37)formatWithPrettier(16-24)
src/handlers/typebox/typebox-type-handlers.ts (1)
src/handlers/typebox/keyof-typeof-handler.ts (1)
KeyOfTypeofHandler(5-90)
src/handlers/typebox/keyof-typeof-handler.ts (2)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)src/handlers/typebox/keyof-type-handler.ts (1)
KeyOfTypeHandler(5-12)
🪛 LanguageTool
docs/overview.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...literal types - keyof typeof expressions - Date type ## Usage ```typescript impor...
(QB_NEW_EN)
🔇 Additional comments (1)
src/handlers/typebox/typebox-type-handlers.ts (1)
10-10: Wiring and precedence look correct.Import, instantiation, and placement before
KeyofTypeHandlerin fallbacks are spot on.Also applies to: 49-49, 94-99
Comment on lines
+14
to
+23
| handle(node: TypeOperatorTypeNode): ts.Expression { | ||
| const typeQuery = node.getTypeNode() | ||
| if (!Node.isTypeQuery(typeQuery)) return makeTypeCall('Any') | ||
|
|
||
| const exprName = typeQuery.getExprName() | ||
| if (!Node.isIdentifier(exprName)) return makeTypeCall('Any') | ||
|
|
||
| const keys = this.getObjectKeys(exprName) | ||
| return keys.length > 0 ? this.createUnion(keys) : makeTypeCall('Any') | ||
| } |
There was a problem hiding this comment.
Empty objects should yield Never, not Any.
keyof {} is never. Your current [] -> Any conflates “resolved empty” with “unresolved”. Return Any only when the target couldn’t be resolved; return Never for resolved-but-empty.
- handle(node: TypeOperatorTypeNode): ts.Expression {
+ handle(node: TypeOperatorTypeNode): ts.Expression {
const typeQuery = node.getTypeNode()
- if (!Node.isTypeQuery(typeQuery)) return makeTypeCall('Any')
+ if (!Node.isTypeQuery(typeQuery)) return makeTypeCall('Any')
const exprName = typeQuery.getExprName()
- if (!Node.isIdentifier(exprName)) return makeTypeCall('Any')
+ if (!Node.isIdentifier(exprName)) return makeTypeCall('Any')
- const keys = this.getObjectKeys(exprName)
- return keys.length > 0 ? this.createUnion(keys) : makeTypeCall('Any')
+ const keys = this.getObjectKeys(exprName)
+ if (keys === null) return makeTypeCall('Any')
+ if (keys.length === 0) return makeTypeCall('Never')
+ return this.createUnion(keys)
}
- private getObjectKeys(node: Node): string[] {
+ private getObjectKeys(node: Node): string[] | null {
const sourceFile = node.getSourceFile()
for (const varDecl of sourceFile.getVariableDeclarations()) {
if (varDecl.getName() === node.getText()) {
return this.extractKeys(varDecl)
}
}
- return []
+ return null
}Also applies to: 25-35
🤖 Prompt for AI Agents
In src/handlers/typebox/keyof-typeof-handler.ts around lines 14-23 (and
similarly for 25-35), the handler currently treats an empty keys array as
unresolved and returns Any; instead, return Never for a resolved-but-empty
object and only return Any when resolution truly failed. Modify getObjectKeys
(or its contract) so it returns a distinct sentinel (e.g., null) when the target
could not be resolved; then update this handler to: if keys === null return
makeTypeCall('Any'); else if keys.length === 0 return makeTypeCall('Never');
otherwise return createUnion(keys). Apply the same fix to the second block at
lines 25-35.
Comment on lines
+75
to
+89
| private createUnion(keys: string[]): ts.Expression { | ||
| const literals = keys.map((key) => { | ||
| const num = Number(key) | ||
| const literal = | ||
| !isNaN(num) && key === String(num) | ||
| ? ts.factory.createNumericLiteral(num) | ||
| : ts.factory.createStringLiteral(key) | ||
|
|
||
| return makeTypeCall('Literal', [literal]) | ||
| }) | ||
|
|
||
| return literals.length === 1 | ||
| ? literals[0]! | ||
| : makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literals)]) | ||
| } |
There was a problem hiding this comment.
Incorrect: numeric object keys should produce string literal key types.
In TypeScript, property names are strings; keyof typeof { 0: 'a' } is "0", not 0. Emitting Type.Literal(0) is semantically wrong.
Apply this diff to always emit string literals for object keys:
- private createUnion(keys: string[]): ts.Expression {
- const literals = keys.map((key) => {
- const num = Number(key)
- const literal =
- !isNaN(num) && key === String(num)
- ? ts.factory.createNumericLiteral(num)
- : ts.factory.createStringLiteral(key)
-
- return makeTypeCall('Literal', [literal])
- })
+ private createUnion(keys: string[]): ts.Expression {
+ const literals = keys.map((key) => {
+ const literal = ts.factory.createStringLiteral(key)
+ return makeTypeCall('Literal', [literal])
+ })📝 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.
Suggested change
| private createUnion(keys: string[]): ts.Expression { | |
| const literals = keys.map((key) => { | |
| const num = Number(key) | |
| const literal = | |
| !isNaN(num) && key === String(num) | |
| ? ts.factory.createNumericLiteral(num) | |
| : ts.factory.createStringLiteral(key) | |
| return makeTypeCall('Literal', [literal]) | |
| }) | |
| return literals.length === 1 | |
| ? literals[0]! | |
| : makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literals)]) | |
| } | |
| private createUnion(keys: string[]): ts.Expression { | |
| const literals = keys.map((key) => { | |
| const literal = ts.factory.createStringLiteral(key) | |
| return makeTypeCall('Literal', [literal]) | |
| }) | |
| return literals.length === 1 | |
| ? literals[0]! | |
| : makeTypeCall('Union', [ts.factory.createArrayLiteralExpression(literals)]) | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/keyof-typeof-handler.ts around lines 75 to 89, the code
currently converts numeric-looking object keys into numeric literals which is
incorrect because object property keys are always strings; update the
createUnion function to always create string literals for keys (i.e. use
ts.factory.createStringLiteral(key) for every key) instead of conditionally
creating NumericLiteral for numeric-looking keys, and keep the rest of the logic
that wraps literals in Type.Literal and combines them with Type.Union unchanged.
Comment on lines
+43
to
+49
| expect(generateFormattedCode(sourceFile)).toBe( | ||
| formatWithPrettier(` | ||
| export const T = Type.Literal(0); | ||
|
|
||
| export type T = Static<typeof T>; | ||
| `), | ||
| ) |
There was a problem hiding this comment.
Adjust expectation: numeric object keys are string key types.
keyof typeof { 0: 'a' } should yield "0".
- export const T = Type.Literal(0);
+ export const T = Type.Literal("0");📝 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.
Suggested change
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Literal(0); | |
| export type T = Static<typeof T>; | |
| `), | |
| ) | |
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Literal("0"); | |
| export type T = Static<typeof T>; | |
| `), | |
| ) |
🤖 Prompt for AI Agents
In tests/handlers/typebox/keyof-typeof.test.ts around lines 43 to 49, the test
expectation uses a numeric literal key (Type.Literal(0)) but JavaScript object
keys that are numeric become string keys, so the expected generated type should
be the string literal "0"; update the expected formatted code to use
Type.Literal("0") (and corresponding Static<typeof T> remains) so the assertion
matches keyof typeof { 0: 'a' } producing the string key type.
Comment on lines
+88
to
+96
| expect(generateFormattedCode(sourceFile)).toBe( | ||
| formatWithPrettier(` | ||
| export const T = Type.Union([ | ||
| Type.Literal(0), | ||
| Type.Literal(1), | ||
| ]); | ||
|
|
||
| export type T = Static<typeof T>; | ||
| `), |
There was a problem hiding this comment.
Adjust expectation for multiple numeric keys: use strings.
- export const T = Type.Union([
- Type.Literal(0),
- Type.Literal(1),
- ]);
+ export const T = Type.Union([
+ Type.Literal("0"),
+ Type.Literal("1"),
+ ]);📝 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.
Suggested change
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Union([ | |
| Type.Literal(0), | |
| Type.Literal(1), | |
| ]); | |
| export type T = Static<typeof T>; | |
| `), | |
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Union([ | |
| Type.Literal("0"), | |
| Type.Literal("1"), | |
| ]); | |
| export type T = Static<typeof T>; | |
| `), |
🤖 Prompt for AI Agents
In tests/handlers/typebox/keyof-typeof.test.ts around lines 88–96, the expected
snapshot currently uses numeric literal types for multiple numeric keys; update
the expectation to use string literal keys instead. Replace the numeric
Type.Literal(0) and Type.Literal(1) (and any corresponding numeric type usages)
with Type.Literal("0") and Type.Literal("1") so the test matches the codegen
behavior that emits numeric keys as strings when there are multiple numeric
keys.
Comment on lines
+112
to
+118
| expect(generateFormattedCode(sourceFile)).toBe( | ||
| formatWithPrettier(` | ||
| export const T = Type.Union([ | ||
| Type.Literal("a"), | ||
| Type.Literal(0), | ||
| ]); | ||
|
|
There was a problem hiding this comment.
Adjust expectation for mixed keys: numeric key should be string literal.
- export const T = Type.Union([
- Type.Literal("a"),
- Type.Literal(0),
- ]);
+ export const T = Type.Union([
+ Type.Literal("a"),
+ Type.Literal("0"),
+ ]);📝 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.
Suggested change
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Union([ | |
| Type.Literal("a"), | |
| Type.Literal(0), | |
| ]); | |
| expect(generateFormattedCode(sourceFile)).toBe( | |
| formatWithPrettier(` | |
| export const T = Type.Union([ | |
| Type.Literal("a"), | |
| Type.Literal("0"), | |
| ]); |
🤖 Prompt for AI Agents
In tests/handlers/typebox/keyof-typeof.test.ts around lines 112 to 118, the test
expectation treats a mixed numeric key as a numeric literal but the generator
returns it as a string literal; update the expected formatted output so the
numeric key is represented as a string literal (e.g. Type.Literal("0")) instead
of a numeric literal, adjusting the snapshot/expect string accordingly and
re-run tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes in this commit add support for handling
keyof typeofexpressions in the TypeBox type handler system. This is an important feature askeyof typeofis a common TypeScript construct used to extract the keys of an object type.The key changes are:
KeyOfTypeofHandlerclass that extends theBaseTypeHandlerand handleskeyof typeofexpressions.KeyOfTypeofHandlerinto the overall handler system, ensuring it is prioritized over the regularKeyOfTypeHandler.keyof typeofhandling, including single string, single numeric, multiple string, multiple numeric, and mixed properties.These changes will allow the TypeBox type handler to correctly generate the appropriate TypeBox types for
keyof typeofexpressions, improving the overall functionality and robustness of the validation schema codegen tool.