feat(typebox): add support for template literal types#4
Conversation
The changes in this commit add support for template literal types in the TypeBox library. The new tests cover various scenarios, including: - One string literal - Multiple string literals - Concatenation with a literal at the start - Concatenation with a literal at the end - Concatenation with a numeric type - Concatenation before and after a string type These changes ensure that the TypeBox library can accurately represent and handle template literal types, which are a powerful feature in TypeScript.
📝 WalkthroughSummary by CodeRabbit
WalkthroughDocs and .gitignore updated; many tests moved/import paths changed and new tests added; an integration test removed. Core code changes: template-literal TypeBox handler now emits structured parts per span, and ts-morph-codegen unifies dependency collection/topological ordering. Minor input-handler cleanup. No public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant G as generateCode (ts-morph-codegen)
participant DC as DependencyCollector
participant TP as TypeAliasParser
participant Out as Emitter
Caller->>G: generateCode(source, exportEverything)
G->>DC: addLocalTypeAliases(localAliases)
G->>DC: collectFromImports(importDecls, exportEverything)
DC-->>G: orderedDependencies (topological)
loop For each ordered dependency
G->>TP: parseWithImportFlag(dep, import=true)
end
alt exportEverything = true
loop all localAliases
G->>TP: parse(localAlias)
end
else
loop localAliases not yet processed
G->>TP: parse(localAlias)
end
end
G->>Out: emit ordered results
Out-->>Caller: formatted code
note over G,DC: Unified flow replaces prior branching behavior
sequenceDiagram
autonumber
participant H as template-literal-type-handler
participant TS as TS TemplateLiteralTypeNode
participant PB as PartBuilder
participant TB as TypeBoxEmitter
H->>TS: read head and spans
alt head literal exists
H->>PB: push Type.Literal(headText)
end
loop For each span
H->>PB: map substitution type to part
alt union of literals
PB-->>H: Type.Union([...Type.Literal(...)])
else number/string keyword
PB-->>H: Type.Number() / Type.String()
else literal type
PB-->>H: Type.Literal(value)
else fallback
PB-->>H: Type.String()
end
H->>PB: push following tail literal (if present)
end
alt parts non-empty
H->>TB: Type.TemplateLiteral(parts)
else
H->>TB: Type.String()
end
TB-->>H: emitted node
note over H,PB: Emits structured array of parts per template span
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/input-handler.ts (1)
45-47: Broken string construction in error messageMismatched quotes mix template literals and single quotes, which will produce a syntax error or unintended output. Replace with a single template literal.
Apply:
- `Multiple resolutions found for path: ${input}. '` + - `Found: ${existingPaths.join(', ')}. ' + - 'Please provide a more specific path.`, + `Multiple resolutions found for path: ${input}. Found: ${existingPaths.join( + ', ', + )}. Please provide a more specific path.`,ARCHITECTURE.md (1)
335-336: Outdated paths: replacetests/ts-morph/...withtests/handlers/typebox/....These two bullets contradict the earlier guidance and the new structure.
- - Run any specific tests with path like `bun test tests/ts-morph/function-types.test.ts` - - Run any specific test cases using command like `bun test tests/ts-morph/function-types.test.ts -t function types` + - Run any specific tests with path like `bun test tests/handlers/typebox/function-types.test.ts` + - Run any specific test cases using a name filter, e.g., `bun test tests/handlers/typebox/function-types.test.ts -t "function types"`
🧹 Nitpick comments (18)
tests/dependency-collector.performance.test.ts (1)
2-2: Import path migration verified — no legacy imports and barrel exports are complete.
- No occurrences of
@test-fixtures/ts-morph/utilsremain.- 13 test files now consistently import from
@test-fixtures/utils.tests/utils.tscorrectly exportscreateSourceFile,formatWithPrettier, andgenerateFormattedCode.Optional: to reduce flakiness of time-based performance tests in CI, consider gating this suite with an environment flag:
// at the top of the file const runPerf = process.env.RUN_PERF_TESTS === '1' const describePerf = runPerf ? describe : describe.skip // then replace `describe(` with `describePerf(`.gitignore (1)
36-36: Prefer directory-ignore with root anchor for clarity.Use a trailing slash (and optionally a root anchor) to avoid unintentionally ignoring files named "samples" outside the project root.
Apply:
-samples +/samples/tests/handlers/typebox/object-types.test.ts (1)
16-24: Ensure Promise-based expectations are awaited or returnedThe tests under
tests/handlers/typebox/object-types.test.ts(and similar suites) use Jest-style.resolvesmatchers without either returning the resulting promise or marking the test callback asasyncand awaiting the expectation. In most runners (including Bun’s built-inbun:test), an unreturned/unawaited promise is not observed, leading to false-positive (silently passing) tests.Changes required for each test using
.resolves:
- Mark the test callback
async.awaittheexpect(...).resolves.toBe(...)call (or alternativelyreturn expect(...).resolves.toBe(...)).Example diff for
tests/handlers/typebox/object-types.test.ts:--- a/tests/handlers/typebox/object-types.test.ts +++ b/tests/handlers/typebox/object-types.test.ts @@ describe('without export', () => { - test('object', () => { + test('object', async () => { const sourceFile = createSourceFile(project, `type A = { a: string }`) - expect(generateFormattedCode(sourceFile)).resolves.toBe( + await expect(generateFormattedCode(sourceFile)).resolves.toBe( formatWithPrettier(` const A = Type.Object({ a: Type.String(), }); type A = Static<typeof A>; `), ) })Apply the same pattern to the other five
test(…)blocks in this file (lines 30, 42, 58, 72, 84) and across all suites that use.resolves.To locate all occurrences at once:
#!/bin/bash # Find .resolves expectations not preceded by 'await' or 'return' rg -nP --type=ts '(?<!(?:await|return)\s)expect\([^)]*\)\.resolves' teststests/handlers/typebox/function-types.test.ts (1)
17-23: Await.resolvesto prevent false positivesSame async expectation concern as other tests; please
awaitorreturnthe Promise from each test case.Apply the pattern:
- expect(generateFormattedCode(sourceFile)).resolves.toBe( + await expect(generateFormattedCode(sourceFile)).resolves.toBe(If you prefer not to mark the test
async, you canreturn expect(...).resolves...instead.Also applies to: 29-36, 41-47, 56-62, 70-76, 85-92, 101-107, 116-122, 129-134, 142-148, 157-163, 172-178
src/input-handler.ts (3)
12-18: Shadowed identifier harms readabilityThe inner
const hasRelativeImportsshadows the outer function name. Rename the inner variable to avoid confusion.-const hasRelativeImports = (sourceFile: SourceFile): boolean => { - const hasRelativeImports = sourceFile +const hasRelativeImports = (sourceFile: SourceFile): boolean => { + const result = sourceFile .getImportDeclarations() .some((importDeclaration) => importDeclaration.isModuleSpecifierRelative()) - return hasRelativeImports + return result }
72-89: Avoid double-creating the same SourceFile; create once with the final virtual pathYou create
temp.ts, then immediately recreate the file atvirtualPath(often alsotemp.ts). This is redundant and can cause unnecessary re-parsing. Create it once at the intended path withoverwrite: trueand return that instance.- if (sourceCode) { + if (sourceCode) { // If callerFile is provided, it means this code came from an existing SourceFile // and relative imports should be allowed - const sourceFile = project.createSourceFile('temp.ts', sourceCode) + const virtualPath = callerFile ? resolve(dirname(callerFile), '__virtual__.ts') : 'temp.ts' + const sourceFile = project.createSourceFile(virtualPath, sourceCode, { overwrite: true }) if (!callerFile && hasRelativeImports(sourceFile)) { throw new Error( 'Relative imports are not supported when providing code as string. ' + 'Only package imports from node_modules are allowed. ' + 'Relative imports will be implemented in the future.', ) } - const virtualPath = callerFile ? resolve(dirname(callerFile), '__virtual__.ts') : 'temp.ts' - - return project.createSourceFile(virtualPath, sourceCode, { overwrite: true }) + return sourceFile }
21-26: Minor: preferstatSync-only existence checks to reduce TOCTOU windowsUsing
existsSyncfollowed bystatSynccan race on very dynamic filesystems. While low risk here, you can simplify by tryingstatSyncin a try/catch and checking.isFile()where needed.If you want, I can provide a small refactor that centralizes this check.
Also applies to: 35-41, 94-99
tests/handlers/typebox/primitive-types.test.ts (1)
16-22: Await the.resolvesexpectationsSame async expectation pattern; ensure each test body awaits or returns the promise so failures don’t slip by.
Example change:
- expect(generateFormattedCode(sourceFile)).resolves.toBe( + await expect(generateFormattedCode(sourceFile)).resolves.toBe(To batch-fix, consider making each affected
testcallbackasyncand prefixing theexpect(...).resolveswithawait.Also applies to: 28-35, 40-46, 52-58, 64-70, 76-82, 102-108, 114-120, 126-132, 138-144, 150-156, 162-168, 174-180
tests/export-everything.test.ts (1)
76-83: Confirm intended ordering contract for unused imported typesThe expected output now places UnusedImportedType after MyType. If this ordering is part of the public/stable generation contract, consider adding a short comment in the test explaining the rationale (topological + local-first) to prevent future regressions.
src/handlers/typebox/template-literal-type-handler.ts (3)
19-28: Head literal extraction: OK; consider simplifying single-literal templatesCurrent behavior emits Type.TemplateLiteral([Type.Literal('foo')]) for a template with no substitutions (e.g., type T =
foo). You can simplify to Type.Literal('foo') in that case to keep output minimal.Apply this small post-processing diff near the return:
- // If no parts were found, fallback to a simple string - if (parts.length === 0) { - return makeTypeCall('String') - } - - // Return TemplateLiteral with array of parts - return makeTypeCall('TemplateLiteral', [ts.factory.createArrayLiteralExpression(parts)]) + // If no parts were found, this is effectively a string + if (parts.length === 0) { + return makeTypeCall('String') + } + // If only a literal part is present, return it directly + if (parts.length === 1) { + return parts[0] + } + // Return TemplateLiteral with array of parts + return makeTypeCall('TemplateLiteral', [ts.factory.createArrayLiteralExpression(parts)])
29-37: Prefer ts-morph APIs over compilerNode access for consistencyYou’re mixing ts-morph (span) with raw TS compiler nodes (span.compilerNode). Using ts-morph’s getters (e.g., span.getTypeNode()) keeps the handler aligned with the rest of the codebase and avoids subtle kind mismatches under different TS versions.
72-76: Literal tail handling: OK; ensure we preserve empty tails when meaningfulCurrent check skips empty strings. That’s fine, but if you find cases where trailing empties are required to trigger TemplateLiteral evaluation, consider preserving them.
src/ts-morph-codegen.ts (1)
69-76: Processing in topological order prevents re-declaration churnThe guard against already-processed types is good. Make sure DependencyCollector.collectFromImports includes local-only dependencies even when there are no imports (your tests imply it does). If not already covered, consider a test where interfaces depend on local type aliases with exportEverything=false.
ARCHITECTURE.md (1)
188-188: Wording nit: “component parts” → “components”.Tighten phrasing and keep terminology consistent.
- - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses template literals into their component parts, handling literal strings, embedded types (string, number, union types), and string/numeric literals. Generates `Type.TemplateLiteral()` calls with arrays of `Type.Literal()`, `Type.String()`, `Type.Number()`, and `Type.Union()` components. + - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses template literals into components, handling literal text, embedded types (string, number, unions), and string/number literals. Generates `Type.TemplateLiteral([...])` using `Type.Literal(...)`, `Type.String()`, `Type.Number()`, and `Type.Union(...)` components.tests/handlers/typebox/advanced-types.test.ts (1)
8-10: Use an in-memory file system for ts-morph projects in tests.Keeps tests hermetic and faster by avoiding disk I/O.
- beforeEach(() => { - project = new Project() - }) + beforeEach(() => { + project = new Project({ useInMemoryFileSystem: true }) + })tests/handlers/typebox/template-literal-types.test.ts (3)
12-22: Consider normalizing a single-literal template toType.Literal(optional).The type
`${'A'}`is equivalent to'A'. EmittingType.Literal('A')instead ofType.TemplateLiteral([Type.Literal('A')])would reduce output verbosity with no loss of fidelity.If you agree, I can adjust the handler to collapse single-part templates and update this test accordingly.
86-88: Quote-style consistency in expectations.Other expectations use single quotes; keep these consistent for readability (Prettier will normalize, but consistency helps).
- Type.Literal("prefix-"), + Type.Literal('prefix-'), @@ - Type.Literal("-suffix"), + Type.Literal('-suffix'),
68-78: Add boolean and literal-number cases for completeness (optional).Template literal types also support
booleanand numeric literal unions. Consider adding:test('concatenated with boolean type', async () => { const sourceFile = createSourceFile(project, 'type T = `flag-${boolean}`;') await expect(generateFormattedCode(sourceFile)).resolves.toBe( formatWithPrettier(` const T = Type.TemplateLiteral([Type.Literal('flag-'), Type.String()]); type T = Static<typeof T>; `), ) }) test('numeric literal union', async () => { const sourceFile = createSourceFile(project, 'type T = `${1|2}`;') await expect(generateFormattedCode(sourceFile)).resolves.toBe( formatWithPrettier(` const T = Type.TemplateLiteral([Type.Union([Type.Literal(1), Type.Literal(2)])]); type T = Static<typeof T>; `), ) })If your handler intentionally stringifies numeric-literal unions to string literals, I can align the expectations to that behavior instead.
📜 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 (20)
.gitignore(1 hunks)ARCHITECTURE.md(4 hunks)src/handlers/typebox/template-literal-type-handler.ts(1 hunks)src/input-handler.ts(1 hunks)src/ts-morph-codegen.ts(2 hunks)tests/dependency-collector.integration.test.ts(1 hunks)tests/dependency-collector.performance.test.ts(1 hunks)tests/dependency-ordering.test.ts(1 hunks)tests/export-everything.test.ts(2 hunks)tests/handlers/typebox/advanced-types.test.ts(1 hunks)tests/handlers/typebox/array-types.test.ts(1 hunks)tests/handlers/typebox/enum-types.test.ts(1 hunks)tests/handlers/typebox/function-types.test.ts(1 hunks)tests/handlers/typebox/object-types.test.ts(1 hunks)tests/handlers/typebox/primitive-types.test.ts(1 hunks)tests/handlers/typebox/template-literal-types.test.ts(1 hunks)tests/handlers/typebox/utility-types.test.ts(1 hunks)tests/import-resolution.test.ts(1 hunks)tests/integration/wikibase/wikibase.ts(0 hunks)tests/ts-morph/advanced-types.test.ts(0 hunks)
💤 Files with no reviewable changes (2)
- tests/ts-morph/advanced-types.test.ts
- tests/integration/wikibase/wikibase.ts
🧰 Additional context used
🧬 Code graph analysis (4)
tests/dependency-ordering.test.ts (1)
tests/utils.ts (2)
generateFormattedCode(17-28)formatWithPrettier(12-15)
src/handlers/typebox/template-literal-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
tests/handlers/typebox/template-literal-types.test.ts (1)
tests/utils.ts (3)
createSourceFile(8-10)generateFormattedCode(17-28)formatWithPrettier(12-15)
tests/handlers/typebox/advanced-types.test.ts (2)
tests/utils.ts (3)
createSourceFile(8-10)generateFormattedCode(17-28)formatWithPrettier(12-15)tests/ts-morph/advanced-types.test.ts (1)
test(38-76)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~166-~166: There might be a mistake here.
Context: ...primitives, arrays, objects, and unions. - : This utility function is responsible f...
(QB_NEW_EN)
[style] ~188-~188: This phrase is redundant. Consider writing “components” or “parts”.
Context: ...`). Parses template literals into their component parts, handling literal strings, embedded typ...
(COMPONENT_PART)
🔇 Additional comments (18)
tests/handlers/typebox/array-types.test.ts (1)
1-1: Unified test-utils import path — looks good.The switch to
@test-fixtures/utilsmatches the PR-wide migration and keeps tests consistent.tests/handlers/typebox/utility-types.test.ts (1)
1-1: Consolidated import path acknowledged.Importing all helpers from
@test-fixtures/utilssimplifies maintenance. No issues spotted.tests/handlers/typebox/enum-types.test.ts (1)
1-1: Import path refactor LGTM.Matches the new convention and should be a no-op for runtime behavior, assuming the barrel re-exports are in place.
tests/dependency-collector.integration.test.ts (2)
2-2: Unified test-fixture import path looks goodSwitching to
@test-fixtures/utilskeeps the symbol the same and reduces churn across tests. No functional impact.
1-4: Path alias for@test-fixtures/*is correctly configured in tsconfig.json
- tsconfig.json (lines 26–29) defines
"paths": { "@test-fixtures/*": ["./tests/*"] }- No
bunfig.tomlorpackage.jsonalias/importsoverrides were found.- Therefore imports like
@test-fixtures/utilsresolve to thetests/utilsdirectory as expected.No further configuration changes are required.
tests/handlers/typebox/object-types.test.ts (1)
1-1: Consolidated utilities import — LGTMImporting from
@test-fixtures/utilsaligns with the rest of the suite and keeps the test helpers centralized.tests/handlers/typebox/function-types.test.ts (1)
1-1: Unified import path — looks goodMatches the new convention; no behavior change.
src/input-handler.ts (1)
87-88: Formatting nit: blank-line removal is fineThe whitespace cleanup is neutral; thanks for keeping it tidy.
tests/handlers/typebox/primitive-types.test.ts (1)
1-1: Import consolidation — approvedConsistent with other test files; no impact beyond module resolution.
tests/import-resolution.test.ts (1)
2-2: Consolidated test-utils import looks goodThe path migration to @test-fixtures/utils is consistent with the rest of the suite; no functional impact on the tests.
tests/export-everything.test.ts (1)
1-1: Import path consolidation LGTMSwitch to @test-fixtures/utils keeps tests consistent and avoids duplicate utility entry points.
tests/dependency-ordering.test.ts (1)
12-38: Great coverage for topological orderingBoth the simple and the longer-chain scenarios effectively assert declaration-before-use. Good use of Prettier-normalized string snapshots to keep expectations stable.
src/ts-morph-codegen.ts (2)
61-67: Unifying dependency collection through a single topological pass is solidAdding local types into the DependencyCollector and then collecting in one pass removes branching and aligns with the new dependency-ordering tests.
76-83: exportEverything=false path: confirm no missed localsYou only sweep remaining locals when exportEverything is true. Given locals are injected into the orderedDependencies graph, that should be fine; just ensure collector always includes locals referenced by downstream constructs (interfaces/functions) so later parsers don’t reference undefined symbols.
ARCHITECTURE.md (2)
302-305: Good update on test commands.Example commands align with the new tests-in-
tests/handlers/typeboxlayout and Bun usage. Nothing else to change here.
272-272: Path Verified:dependency-collector.performance.test.tsExistsThe file
tests/dependency-collector.performance.test.tsis present in the repository, so the reference inARCHITECTURE.mdis correct.tests/handlers/typebox/advanced-types.test.ts (1)
1-5: Nice, cohesive test harness.Consistent use of shared utils (
@test-fixtures/utils) and Bun’s test API improves readability and keeps snapshots stable via Prettier.tests/handlers/typebox/template-literal-types.test.ts (1)
24-66: Great coverage of core scenarios for template literals.The cases cover single/union literals, head/tail concatenation, numeric type interpolation, and multiple spans—solid baseline.
Also applies to: 68-94
| export const DataValueByDataType = Type.Object({ | ||
| "'string'": StringSnakDataValue, | ||
| "'commonsMedia'": CommonsMediaSnakDataValue, | ||
| "'external-id'": ExternalIdSnakDataValue, | ||
| "'geo-shape'": GeoShapeSnakDataValue, | ||
| }); | ||
|
|
||
| export type DataValueByDataType = Static<typeof DataValueByDataType>; | ||
| `), |
There was a problem hiding this comment.
Property keys appear double-quoted (likely unintended semantics)
In the expected Type.Object for DataValueByDataType, keys are emitted as "'string'", "'commonsMedia'", etc. This encodes the single quotes into the property names, producing keys like literally 'string' instead of string. That’s almost certainly not the intended runtime shape.
Recommend normalizing string-literal keys to their text (without surrounding quotes) in the generator, and updating this test expectation accordingly.
Apply this diff to the test expectation (after fixing the generator):
- export const DataValueByDataType = Type.Object({
- "'string'": StringSnakDataValue,
- "'commonsMedia'": CommonsMediaSnakDataValue,
- "'external-id'": ExternalIdSnakDataValue,
- "'geo-shape'": GeoShapeSnakDataValue,
- });
+ export const DataValueByDataType = Type.Object({
+ 'string': StringSnakDataValue,
+ 'commonsMedia': CommonsMediaSnakDataValue,
+ 'external-id': ExternalIdSnakDataValue,
+ 'geo-shape': GeoShapeSnakDataValue,
+ });Follow-up generator fix (high level): when turning a ts.StringLiteral key into a property name, use its .text (no quotes) rather than .getText() or raw source. If the current implementation already does this in one place, check for a code path that still uses getText for index signatures or mapped types.
📝 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.
| export const DataValueByDataType = Type.Object({ | |
| "'string'": StringSnakDataValue, | |
| "'commonsMedia'": CommonsMediaSnakDataValue, | |
| "'external-id'": ExternalIdSnakDataValue, | |
| "'geo-shape'": GeoShapeSnakDataValue, | |
| }); | |
| export type DataValueByDataType = Static<typeof DataValueByDataType>; | |
| `), | |
| export const DataValueByDataType = Type.Object({ | |
| 'string': StringSnakDataValue, | |
| 'commonsMedia': CommonsMediaSnakDataValue, | |
| 'external-id': ExternalIdSnakDataValue, | |
| 'geo-shape': GeoShapeSnakDataValue, | |
| }); | |
| export type DataValueByDataType = Static<typeof DataValueByDataType>; | |
| `), |
| expect(generateFormattedCode(sourceFile)).resolves.toBe( | ||
| formatWithPrettier(` | ||
| const A = myVar; | ||
|
|
||
| type A = Static<typeof A>; | ||
| `), | ||
| ) |
There was a problem hiding this comment.
Await your promise-based expectations; current tests may pass without asserting.
expect(promise).resolves must be awaited or returned; otherwise Bun/Jest will not wait for resolution and assertions can be skipped.
Apply this diff:
- test('typeof variable', () => {
+ test('typeof variable', async () => {
const sourceFile = createSourceFile(
project,
`
const myVar = { x: 1, y: 'hello' }
type A = typeof myVar
`,
)
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
formatWithPrettier(`
const A = myVar;
type A = Static<typeof A>;
`),
)
})
@@
- test('typeof with qualified name', () => {
+ test('typeof with qualified name', async () => {
const sourceFile = createSourceFile(
project,
`
namespace MyNamespace {
export const config = { port: 3000 }
}
type A = typeof MyNamespace.config
`,
)
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
formatWithPrettier(`
const A = MyNamespace_config;
type A = Static<typeof A>;
`),
)
})Also applies to: 42-48
🤖 Prompt for AI Agents
In tests/handlers/typebox/advanced-types.test.ts around lines 22-28 (and
similarly 42-48) the test uses expect(promise).resolves without awaiting or
returning the assertion, so the test runner may not wait for resolution; update
each test to await the promise-based expectation (e.g., add "await" before
expect(...).resolves.toBe(...)) or return the expect call, and ensure the
surrounding test function is declared async so the await is valid.
| project = new Project() | ||
| }) | ||
|
|
||
| test('one string', () => { |
There was a problem hiding this comment.
Await your promise-based expectations in every test.
Same issue as in advanced-types: without await/return, assertions may be skipped.
Apply this diff:
- test('one string', () => {
+ test('one string', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
@@
- test('multiple strings', () => {
+ test('multiple strings', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
@@
- test('concatenated with literal at start', () => {
+ test('concatenated with literal at start', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
@@
- test('concatenated with literal at end', () => {
+ test('concatenated with literal at end', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
@@
- test('concatenated with numeric type', () => {
+ test('concatenated with numeric type', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(
@@
- test('concatenation before and after', () => {
+ test('concatenation before and after', async () => {
@@
- expect(generateFormattedCode(sourceFile)).resolves.toBe(
+ await expect(generateFormattedCode(sourceFile)).resolves.toBe(Also applies to: 15-21, 24-24, 27-35, 38-38, 41-49, 53-53, 56-64, 68-68, 71-77, 80-80, 83-93
🤖 Prompt for AI Agents
In tests/handlers/typebox/template-literal-types.test.ts around lines 12, 15-21,
24, 27-35, 38, 41-49, 53, 56-64, 68, 71-77, 80, and 83-93 the test cases use
promise-based Jest expectations but do not await or return them, which can let
assertions be skipped; update each test to either prepend await (inside an async
test) or return the promise so the test runner waits for resolution (e.g., make
the test callback async and use await expect(...).resolves/rejects... or return
expect(...).resolves/rejects...), applying this change consistently to all
listed test blocks.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
ARCHITECTURE.md (1)
191-191: Fix handler classification:typeofis not a TypeOperator; it’s handled as a TypeQuery.This repeats a prior note; keep
type-operator-handlertokeyof/readonly/uniqueand referencetypeof-type-handler.tsfortypeof.Apply:
- - <mcfile name="type-operator-handler.ts" path="src/handlers/typebox/type-operator-handler.ts"></mcfile>: Handles TypeScript type operators like `keyof`, `typeof`. + - <mcfile name="type-operator-handler.ts" path="src/handlers/typebox/type-operator-handler.ts"></mcfile>: Handles TypeScript type operators like `keyof`, `readonly`, and `unique` (not `typeof`; see `typeof-type-handler.ts`).
🧹 Nitpick comments (4)
src/input-handler.ts (1)
74-78: Avoid cross-call collisions: make the virtual filename unique (hash-based).Using a fixed
__virtual__.ts/temp.tscan collide when multiple in-memory files are created within the same Project. A short content hash in the filename prevents accidental overwrite and makes debugging easier.Apply:
- const virtualPath = callerFile ? resolve(dirname(callerFile), '__virtual__.ts') : 'temp.ts' - const sourceFile = project.createSourceFile(virtualPath, sourceCode, { + const hash = createHash('sha256').update(sourceCode).digest('hex').slice(0, 8) + const virtualPath = callerFile + ? resolve(dirname(callerFile), `__virtual__.${hash}.ts`) + : resolve(process.cwd(), `.typebox-virtual/${hash}.ts`) + const sourceFile = project.createSourceFile(virtualPath, sourceCode, { overwrite: true, })Add this import at the top of the file:
import { createHash } from 'crypto'ARCHITECTURE.md (3)
166-170: Nit: tighten types and wording (“ts.Expression”, assertive voice).
getTypeBoxTypereturnsts.Expressionin code. Also aligns tone across bullets.Apply:
- - <mcfile name="typebox-call.ts" path="src/utils/typebox-call.ts"></mcfile>: Contains the core logic for converting TypeScript type nodes into TypeBox `Type` expressions. `getTypeBoxType` takes a `TypeNode` as input and returns a `ts.Node` representing the equivalent TypeBox schema. + - <mcfile name="typebox-call.ts" path="src/utils/typebox-call.ts"></mcfile>: Converts TypeScript type nodes into TypeBox `Type` expressions. `getTypeBoxType` maps a `TypeNode` to a `ts.Expression` representing the equivalent TypeBox schema. - - <mcfile name="add-static-type-alias.ts" path="src/utils/add-static-type-alias.ts"></mcfile>: Generates and adds the `export type [TypeName] = Static<typeof [TypeName]>` declaration to the output source file. This declaration is essential for enabling TypeScript's static type inference from the dynamically generated TypeBox schemas, ensuring type safety at compile time. + - <mcfile name="add-static-type-alias.ts" path="src/utils/add-static-type-alias.ts"></mcfile>: Generates and inserts `export type [TypeName] = Static<typeof [TypeName]>` to enable static type inference. - - <mcfile name="typebox-codegen-utils.ts" path="src/utils/typebox-codegen-utils.ts"></mcfile>: Contains general utility functions that support the TypeBox code generation process, such as helper functions for string manipulation or AST node creation. + - <mcfile name="typebox-codegen-utils.ts" path="src/utils/typebox-codegen-utils.ts"></mcfile>: Provides shared helpers for code generation (e.g., string utilities, AST node builders). - - <mcfile name="typescript-ast-parser.ts" path="src/utils/typescript-ast-parser.ts"></mcfile>: Responsible for parsing TypeScript source code and extracting relevant Abstract Syntax Tree (AST) information. It provides functions to navigate the AST and identify specific nodes like type aliases, interfaces, or enums. + - <mcfile name="typescript-ast-parser.ts" path="src/utils/typescript-ast-parser.ts"></mcfile>: Parses TypeScript source and exposes utilities for navigating the AST and locating declarations (type aliases, interfaces, enums). - - <mcfile name="typescript-ast-types.ts" path="src/utils/typescript-ast-types.ts"></mcfile>: Defines custom types and interfaces that represent the structured AST information extracted by `typescript-ast-parser.ts`, providing a consistent data model for further processing. + - <mcfile name="typescript-ast-types.ts" path="src/utils/typescript-ast-types.ts"></mcfile>: Defines internal types and interfaces for the structured AST consumed by the generators.
188-188: Clarify scope of template-literal support to match current handler behavior.Explicitly note that embedded types supported are
string,number, string/numeric literal unions; other substitutions degrade tostring.Apply:
- - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses template literals into components, handling literal text, embedded types (string, number, unions), and string/numeric literals. + - <mcfile name="template-literal-type-handler.ts" path="src/handlers/typebox/template-literal-type-handler.ts"></mcfile>: Handles TypeScript template literal types (e.g., `` `hello-${string}` ``). Parses into literal text and embedded types (`string`, `number`, and unions of string/numeric literals). Unsupported substitutions degrade to `string`.
334-335: Grammar nits: add articles.Minor readability polish.
Apply:
-- Run any specific tests with path like `bun test tests/handlers/typebox/function-types.test.ts` -- Run any specific test cases using command like `bun test tests/handlers/typebox/function-types.test.ts -t function types` +- Run any specific tests with a path like `bun test tests/handlers/typebox/function-types.test.ts` +- Run any specific test cases using a command like `bun test tests/handlers/typebox/function-types.test.ts -t "function types"`
📜 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 (3)
ARCHITECTURE.md(5 hunks)src/handlers/typebox/template-literal-type-handler.ts(1 hunks)src/input-handler.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/handlers/typebox/template-literal-type-handler.ts (1)
src/utils/typebox-codegen-utils.ts (1)
makeTypeCall(3-12)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~166-~166: There might be a mistake here.
Context: ...resenting the equivalent TypeBox schema. - : Generates and adds the `export type [T...
(QB_NEW_EN)
[grammar] ~191-~191: There might be a mistake here.
Context: ...t type operators like keyof, typeof. - : Handles references to other types (e.g...
(QB_NEW_EN)
[grammar] ~334-~334: There might be a mistake here.
Context: ...itting changes - Run any specific tests with path like `bun test tests/handlers/type...
(QB_NEW_EN)
[grammar] ~335-~335: There might be a mistake here.
Context: ....test.ts- Run any specific test cases using command likebun test tests/handlers/t...
(QB_NEW_EN)
🔇 Additional comments (4)
src/input-handler.ts (3)
13-16: LGTM: direct boolean return simplifieshasRelativeImports.The refactor is clear and keeps intent obvious.
43-46: Error message cleanup improves clarity.Removing stray quotes and normalizing spacing reads better for users diagnosing ambiguous resolutions.
79-85: Good guardrail on relative imports for string inputs.The check keeps string-based usage safe until relative import resolution is fully supported without a
callerFilecontext.src/handlers/typebox/template-literal-type-handler.ts (1)
35-76: Tests couldn’t run—manual verification needed
It looks like thebuntest runner isn’t available in this environment (command not found), so the proposed tests for boolean, bigint, and union literal handling didn’t execute. Please verify that the new substitution logic works as intended by running your project’s test suite with your usual test runner (e.g.npm test,yarn test, orpnpm test) and targeting the following scenarios:•
${true | false}→ should resolve toType.Union([Literal('true'), Literal('false')])
•${42 | 7}→ should resolve toType.Union([Literal('42'), Literal('7')])
•${T}whereTis a union alias of literal types → should inline the literals or fallback toType.String()if mixedOnce you’ve confirmed that those tests pass (or adjusted the scripts to use your available runner), let me know or push updates so we can close out this refactor.
| // Add the head part (literal string before first substitution) | ||
| const head = typeNode.getHead() | ||
| const headCompilerNode = head.compilerNode as ts.TemplateHead | ||
| const headText = headCompilerNode.text | ||
| if (headText) { | ||
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(headText)])) | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: Dropping empty head breaks TemplateLiteral shape; always include the head (even empty).
TypeBox TemplateLiteral parts should start with a literal segment. Skipping an empty head can lead to arrays starting with a type, which can be rejected and alters semantics.
Apply:
- const headText = headCompilerNode.text
- if (headText) {
- parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(headText)]))
- }
+ const headText = headCompilerNode.text
+ parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(headText)]))📝 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.
| // Add the head part (literal string before first substitution) | |
| const head = typeNode.getHead() | |
| const headCompilerNode = head.compilerNode as ts.TemplateHead | |
| const headText = headCompilerNode.text | |
| if (headText) { | |
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(headText)])) | |
| } | |
| // Add the head part (literal string before first substitution) | |
| const head = typeNode.getHead() | |
| const headCompilerNode = head.compilerNode as ts.TemplateHead | |
| const headText = headCompilerNode.text | |
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(headText)])) |
🤖 Prompt for AI Agents
In src/handlers/typebox/template-literal-type-handler.ts around lines 21 to 28,
the code currently skips adding a head Literal when headText is empty which
breaks the TemplateLiteral shape; always push a Literal head (use empty string
when headText is falsy) so the parts array begins with a literal segment
regardless of content; update the logic to create and push
makeTypeCall('Literal', [ts.factory.createStringLiteral(headText || '')])
unconditionally.
| // Add the literal part after the substitution | ||
| const literalText = compilerNode.literal?.text | ||
| if (literalText) { | ||
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(literalText)])) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t drop empty trailing literals; always append span literal text.
compilerNode.literal.text can be an empty string; truthy checks erase it, disturbing the expected alternating literal/type pattern.
Apply:
- const literalText = compilerNode.literal?.text
- if (literalText) {
- parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(literalText)]))
- }
+ const literalText = compilerNode.literal.text
+ parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(literalText)]))📝 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.
| // Add the literal part after the substitution | |
| const literalText = compilerNode.literal?.text | |
| if (literalText) { | |
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(literalText)])) | |
| } | |
| } | |
| // Add the literal part after the substitution | |
| const literalText = compilerNode.literal.text | |
| parts.push(makeTypeCall('Literal', [ts.factory.createStringLiteral(literalText)])) | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/template-literal-type-handler.ts around lines 78 to 83,
the current truthy check for compilerNode.literal?.text drops empty string
literals and breaks the literal/type alternation; change the condition to
explicitly check for undefined (e.g., if (compilerNode.literal &&
compilerNode.literal.text !== undefined) or if (compilerNode.literal?.text !==
undefined)) and always push the Literal span using that text (allowing empty
string) so trailing empty literals are preserved.
| // If no parts were found, fallback to a simple string | ||
| if (parts.length === 0) { | ||
| return makeTypeCall('String') | ||
| } | ||
|
|
||
| // Return TemplateLiteral with array of parts | ||
| return makeTypeCall('TemplateLiteral', [ts.factory.createArrayLiteralExpression(parts)]) |
There was a problem hiding this comment.
Empty template should be Literal(''), and ensure first part is a Literal.
- For `` type literals (empty template), returning
Type.String()is too wide; the exact type is the empty string. - Defensive: if earlier logic ever yields a non-literal as first part, prepend
Literal('')to preserve valid TemplateLiteral structure.
Apply:
- // If no parts were found, fallback to a simple string
- if (parts.length === 0) {
- return makeTypeCall('String')
- }
+ // If no parts were found, return the exact empty string literal
+ if (parts.length === 0) {
+ return makeTypeCall('Literal', [ts.factory.createStringLiteral('')])
+ }
+
+ // Ensure first part is a Literal to satisfy TemplateLiteral contract
+ const first = parts[0]
+ const firstIsLiteral =
+ ts.isCallExpression(first) &&
+ ts.isPropertyAccessExpression(first.expression) &&
+ first.expression.expression.getText() === 'Type' &&
+ first.expression.name.getText() === 'Literal'
+ if (!firstIsLiteral) {
+ parts.unshift(makeTypeCall('Literal', [ts.factory.createStringLiteral('')]))
+ }📝 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 no parts were found, fallback to a simple string | |
| if (parts.length === 0) { | |
| return makeTypeCall('String') | |
| } | |
| // Return TemplateLiteral with array of parts | |
| return makeTypeCall('TemplateLiteral', [ts.factory.createArrayLiteralExpression(parts)]) | |
| // If no parts were found, return the exact empty string literal | |
| if (parts.length === 0) { | |
| return makeTypeCall('Literal', [ts.factory.createStringLiteral('')]) | |
| } | |
| // Ensure first part is a Literal to satisfy TemplateLiteral contract | |
| const first = parts[0] | |
| const firstIsLiteral = | |
| ts.isCallExpression(first) && | |
| ts.isPropertyAccessExpression(first.expression) && | |
| first.expression.expression.getText() === 'Type' && | |
| first.expression.name.getText() === 'Literal' | |
| if (!firstIsLiteral) { | |
| parts.unshift(makeTypeCall('Literal', [ts.factory.createStringLiteral('')])) | |
| } | |
| // Return TemplateLiteral with array of parts | |
| return makeTypeCall('TemplateLiteral', [ts.factory.createArrayLiteralExpression(parts)]) |
🤖 Prompt for AI Agents
In src/handlers/typebox/template-literal-type-handler.ts around lines 85 to 91,
change the fallback for an empty template so it returns a Literal('') instead of
String(), and ensure the returned TemplateLiteral always has a string Literal as
its first part: if parts.length === 0 return a Literal for the empty string;
otherwise, if parts[0] is not a Literal, prepend a Literal('') to parts before
calling makeTypeCall('TemplateLiteral',
[ts.factory.createArrayLiteralExpression(parts)]), so the TemplateLiteral
structure is valid and the first element is always a Literal.
1 participant
The changes in this commit add support for template literal types in the TypeBox library. The new tests cover various scenarios, including:
These changes ensure that the TypeBox library can accurately represent and handle template literal types, which are a powerful feature in TypeScript.