feat(traverse): improve type dependency analysis #11
Conversation
Improve the type dependency analysis by: - Using a dedicated `DependencyAnalyzer` class to handle the analysis - Extracting interface references from type aliases and type alias references from interfaces - Determining the correct processing order for interfaces and type aliases This ensures that the code generation process can handle complex type dependencies correctly, allowing for more robust and reliable schema generation.
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces cross-dependency analysis between interfaces and type aliases via new AST traversal and a DependencyAnalyzer, updates ts-morph-codegen control flow to process in conditional phases, enhances TypeBox type-reference handling for generic arguments, refactors generic constraint creation to TSchema nodes, relocates dependency modules from utils to traverse, and updates docs/tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Codegen as ts-morph-codegen
participant Analyzer as DependencyAnalyzer
participant Traversal as ASTTraversal
User->>Codegen: generate(...)
Codegen->>Analyzer: analyzeProcessingOrder(typeAliases, interfaces)
Analyzer->>Traversal: extract refs (both directions)
Traversal-->>Analyzer: {ifaceRefs, aliasRefs}
Analyzer-->>Codegen: {processInterfacesFirst, aliasesDependingOnIfaces, ifacesDependingOnAliases}
alt Only ifaces depend on aliases
Codegen->>Codegen: Process aliases (needed), then interfaces, then remaining aliases
else Only aliases depend on ifaces
Codegen->>Codegen: getInterfaceProcessingOrder(...)
Codegen->>Codegen: Process interfaces first, then aliases
else Both directions
rect rgba(200, 230, 255, 0.3)
note right of Codegen: Three-phase processing
Codegen->>Codegen: Phase A: aliases that interfaces depend on
Codegen->>Codegen: Phase B: interfaces
Codegen->>Codegen: Phase C: aliases depending on interfaces
end
end
Codegen-->>User: Generated artifacts
sequenceDiagram
autonumber
participant TRH as TypeReferenceHandler
participant TB as TypeBox helpers
TRH->>TRH: handle(TypeReferenceNode)
alt Identifier with type args
TRH->>TB: getTypeBoxType(arg) x N
TB-->>TRH: TypeBox arg nodes
TRH->>TRH: Emit call expr: Callee(TypeBoxArgs...)
else Identifier without args
TRH->>TRH: Emit identifier
else Non-identifier
TRH->>TRH: makeTypeCall('Any')
end
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tests/dependency-collector.performance.test.ts (1)
10-13: Clear global caches in this suite to avoid cross-Project stale ts-morph NodesStatic caches in DependencyCollector may retain Nodes from previous Projects; without clearing, performance tests (which construct fresh Projects) can read cached Nodes tied to a different Project, causing subtle correctness and perf anomalies. Clear the cache in beforeEach here as well (you already do this in other suites).
Apply this diff:
beforeEach(() => { + DependencyCollector.clearGlobalCache() project = new Project() collector = new DependencyCollector() })src/traverse/dependency-collector.ts (2)
21-25: Static cache stores ts-morph Nodes across Projects — risk of cross-Project contamination
fileCacheis static and caches arrays ofImportDeclaration/TypeAliasDeclarationNodes. When tests or consumers create newProjectinstances, the same file path (e.g., "file0.ts") can map to cached Nodes from a different Project. Reusing those Nodes against a new Project is unsafe and can produce wrong results or nondeterministic behavior. You already mitigate in some suites by callingclearGlobalCache(), but making correctness depend on disciplined external cache clearing is brittle.Two actionable options:
- Preferred: make
fileCacheinstance-scoped (per collector), which naturally ties cache to a single Project; keep the static clear method for backward compatibility but have it only clear other global caches.- Alternative: keep a static cache but never store ts-morph Nodes; cache only plain, project-agnostic data (e.g., type names, import specifiers) and re-resolve Nodes from the current
SourceFile.Apply this diff to implement the preferred option and also clear the reference-extractor cache from the static clear:
export class DependencyCollector { private dependencies = new Map<string, TypeDependency>() private visited = new Set<string>() private visitedFiles = new Set<string>() - private static fileCache = new Map< + private readonly fileCache = new Map< string, { imports: ImportDeclaration[]; types: TypeAliasDeclaration[] } >() @@ // Check cache for file data - let fileData = DependencyCollector.fileCache.get(filePath) + let fileData = this.fileCache.get(filePath) if (!fileData) { fileData = { imports: this.fileResolver.getImportDeclarations(importedSourceFile), types: this.fileResolver.getTypeAliases(importedSourceFile), } - DependencyCollector.fileCache.set(filePath, fileData) + this.fileCache.set(filePath, fileData) } @@ static clearGlobalCache(): void { - DependencyCollector.fileCache.clear() + // Now only clears global extractor cache; per-instance caches are tied to a single Project/collector. + DefaultTypeReferenceExtractor.clear() } }Follow-up: if you prefer to keep the static cache, I can provide a variant that caches only POJOs and rebinds Nodes per-Project.
Also applies to: 67-77, 206-209
165-172: Topological sort uses stale “visited” across runs
visitedis never reset betweencollectFromImportscalls. Subsequent calls in the same instance can return incomplete orderings because already visited types are skipped. Reset at the start of each sort.Apply this diff:
private topologicalSort(): TypeDependency[] { + // Ensure deterministic ordering for each invocation + this.visited.clear() const result: TypeDependency[] = [] const visiting = new Set<string>() const dependencyKeys = new Set(this.dependencies.keys())src/traverse/dependency-type.ts (1)
11-17: Cache key ignores dependency set; extraction filters by dependencies — may return stale/incomplete refsThe static cache is keyed solely by
typeNode.getText(), but the extraction filters out names not present in the provideddependenciesMap. If the set of dependencies grows between calls (common during collection), cached results can be missing references. Also, returning only names present independenciescouples extraction to caller state and complicates safe caching.Refactor to:
- Extract all referenced type names (caller can filter as needed).
- Deduplicate via a Set.
- Add a static clear method for test isolation and to support
DependencyCollector.clearGlobalCache().Apply this diff:
export class DefaultTypeReferenceExtractor implements TypeReferenceExtractor { private static cache = new Map<string, string[]>() extractTypeReferences(typeNode: Node, dependencies: Map<string, TypeDependency>): string[] { // Simple cache key using content only const cacheKey = typeNode.getText() const cached = DefaultTypeReferenceExtractor.cache.get(cacheKey) if (cached) return cached - const references: string[] = [] + const references = new Set<string>() const visited = new Set<Node>() const traverse = (node: Node): void => { if (visited.has(node)) return visited.add(node) // Handle type references if (Node.isTypeReference(node)) { const typeRefNode = node as TypeReferenceNode - const typeName = typeRefNode.getTypeName().getText() - - if (dependencies.has(typeName)) { - references.push(typeName) - } - return // No need to traverse children of type references + // Capture the textual name; if qualified (e.g., NS.Type), use the right-most identifier + const raw = typeRefNode.getTypeName().getText() + const typeName = raw.includes('.') ? raw.split('.').pop()! : raw + references.add(typeName) + return // No need to traverse children of type references } // Use forEachChild for better performance instead of getChildren() node.forEachChild(traverse) } traverse(typeNode) // Cache the result - DefaultTypeReferenceExtractor.cache.set(cacheKey, references) - return references + const result = Array.from(references) + DefaultTypeReferenceExtractor.cache.set(cacheKey, result) + return result } + // Static clear for global test/consumer resets + static clear(): void { + DefaultTypeReferenceExtractor.cache.clear() + } + clear(): void { - DefaultTypeReferenceExtractor.cache.clear() + DefaultTypeReferenceExtractor.clear() } }Also applies to: 21-34, 36-45
ARCHITECTURE.md (1)
68-71: Correct the inheritance composition primitive toType.CompositeDocumentation still mentions
Type.Intersect, while the implementation and tests useType.Composite([...]). Update for accuracy.-7. **Interface Processing**: ... Interfaces that extend other interfaces are generated using `Type.Intersect` to combine the base interface with additional properties, ensuring proper inheritance semantics. +7. **Interface Processing**: ... Interfaces that extend other interfaces are generated using `Type.Composite` to combine the base interface with additional properties, ensuring proper inheritance semantics.
🧹 Nitpick comments (17)
tests/dependency-collector.performance.test.ts (1)
35-40: Guard against timing-based flakiness in CIHard thresholds (e.g., <1s, <2s, <3s, <5s) can intermittently fail on slower CI machines. Consider using wider margins in CI, environment-gated thresholds, or assertions on complexity (e.g., relative growth between N and 2N) instead of absolute times.
Would you like me to convert these to complexity-style checks or add CI-aware thresholds?
Also applies to: 64-69, 124-128, 162-167, 215-220, 257-262, 293-298, 334-339
src/traverse/dependency-collector.ts (1)
86-96: Type-name-only keys risk collisions across modulesDependencies are keyed solely by
typeAlias.getName(). If two imported files export the same alias name, one silently wins and the other is dropped. That can mis-resolve references and produce incorrect schemas.Options:
- Key by composite "{filePath}#{typeName}" and introduce a name-to-key map for lookups in the extractor/toposort.
- Or detect duplicates and throw with a clear diagnostic to force callers to alias imports.
Do we have scenarios importing homonymous types from different modules? If yes, I can wire the composite-key approach and adapt the extractor/toposort accordingly.
tests/dependency-collector.integration.test.ts (1)
10-14: Nice: clearing the global cache stabilizes multi-Project test runsSince we’ll also clear the extractor cache (see suggested code changes), consider clearing it here for full isolation.
If you accept the extractor change, add:
+import { DefaultTypeReferenceExtractor } from '@daxserver/validation-schema-codegen/traverse/dependency-type' @@ beforeEach(() => { project = new Project() collector = new DependencyCollector() DependencyCollector.clearGlobalCache() + DefaultTypeReferenceExtractor.clear() })tests/import-resolution.test.ts (1)
9-12: Consider clearing the extractor cache too for full test isolationOnce we add a static
clear()on the extractor, mirror the cache clear here as well (see integration test suggestion).tests/dependency-collector.unit.test.ts (1)
167-192: Strengthen the “topological sort integration” test with behavior assertions.Right now the test only checks the size of the dependency map. Consider asserting that the extractor was called with the expected TypeNode and that the ordering/topology is respected (or, at minimum, that the dependency for TypeA includes ReferencedType). Example additions:
- expect(mockTypeReferenceExtractor.extractTypeReferences).toHaveBeenCalledTimes(1)
- expect([...collector.getDependencies().keys()]).toEqual(expect.arrayContaining(['TypeA','ReferencedType']))
I can draft the additional assertions if you share the intended observable signals of the topo sort.
src/parsers/parse-interfaces.ts (1)
110-117: Refactor to constraintNode is fine; ensure TSchema is imported where this alias is emitted.Using a local variable improves readability. Please confirm the generated file also contains a type-only import for TSchema from @sinclair/typebox; otherwise the emitted type parameter constraint will fail type-checking.
If it helps, I can add a small helper to guarantee “import type { TSchema } from '@sinclair/typebox'” is present when generics are detected.
src/traverse/ast-traversal.ts (3)
19-22: Return copies from cache to avoid accidental external mutation.Cached arrays are returned by reference. If a caller accidentally mutates the array, you’ll corrupt the cache. Return a defensive copy for safety.
- const cached = ASTTraversal.cache.get(cacheKey) - if (cached) return cached + const cached = ASTTraversal.cache.get(cacheKey) + if (cached) return [...cached]
100-103: Apply the same dedupe and defensive-copy policy on cache set/get.Mirror the Set + copy approach to keep behavior consistent across both extractors and avoid duplicate names.
- ASTTraversal.cache.set(cacheKey, references) - return references + const out = Array.from(new Set(references)) + ASTTraversal.cache.set(cacheKey, [...out]) + return out
109-111: Expose cache lifecycle in higher-level flows.clearCache() exists but nothing here calls it. Ensure the dependency analysis entry points invoke this between independent program runs to avoid stale results across files/projects.
src/handlers/typebox/object/interface-type-handler.ts (1)
72-74: Good: explicit constraintNode improves clarity and parity with parsers.Small nit: double-check that the generator ensures an import type { TSchema } from '@sinclair/typebox' is present whenever a generic interface is emitted. Otherwise, consumers may hit missing identifier errors.
src/handlers/typebox/type-reference-handler.ts (1)
29-33: Fallback to Any for non-identifiers may be too lossy.Consider returning a property access expression for qualified names without type arguments as well (mirroring the call-site logic), instead of defaulting to Any. This preserves references like ns.Foo.
Please confirm there are dedicated handlers for well-known built-ins (e.g., Array, Record, Partial, Promise). Otherwise this handler will emit runtime calls (e.g., Promise(Type.String())) that likely aren’t desired in the TypeBox path.
src/ts-morph-codegen.ts (3)
79-91:processInterfacesFirstis computed but unused; simplify or use consistentlyYou compute
processInterfacesFirstinsideanalyzeProcessingOrderbut branch onhasInterfacesDependingOnTypeAliases/hasTypeAliasesDependingOnInterfacesinstead. Either honorprocessInterfacesFirstto drive the flow or drop it from the API to reduce cognitive overhead.Apply this minimal diff to avoid carrying unused state here:
- const dependencyAnalysis = dependencyAnalyzer.analyzeProcessingOrder(localTypeAliases, interfaces) + const dependencyAnalysis = dependencyAnalyzer.analyzeProcessingOrder(localTypeAliases, interfaces) + // Prefer set lookups over repeated Array.includes + const typeAliasesDependingOnInterfacesSet = new Set(dependencyAnalysis.typeAliasesDependingOnInterfaces) + const interfacesDependingOnTypeAliasesSet = new Set(dependencyAnalysis.interfacesDependingOnTypeAliases) - const hasInterfacesDependingOnTypeAliases = - dependencyAnalysis.interfacesDependingOnTypeAliases.length > 0 - const hasTypeAliasesDependingOnInterfaces = - dependencyAnalysis.typeAliasesDependingOnInterfaces.length > 0 + const hasInterfacesDependingOnTypeAliases = interfacesDependingOnTypeAliasesSet.size > 0 + const hasTypeAliasesDependingOnInterfaces = typeAliasesDependingOnInterfacesSet.size > 0
113-121: Replace O(n) membership checks with Set lookups
includeson arrays is used in tight loops. Convert to set membership for linear-to-constant-time lookups.Apply:
- const dependsOnInterface = dependencyAnalysis.typeAliasesDependingOnInterfaces.includes( - dependency.typeAlias.getName(), - ) + const dependsOnInterface = typeAliasesDependingOnInterfacesSet.has( + dependency.typeAlias.getName() + )
123-148: Avoid per-iteration Map construction and reduce repeated AST scansInside the Case 3 pre-pass you rebuild
new Map(localTypeAliases.map(...))and re-extract refs for every dependency. Hoist constants and precompute interface→typeAlias references once to reduce complexity from O(n*m) to O(n+m).Proposed refactor within this block:
- orderedDependencies.forEach((dependency) => { - const interfaceDependsOnThis = dependencyAnalysis.interfacesDependingOnTypeAliases.some( - (interfaceName) => { - const interfaceDecl = interfaces.find((i) => i.getName() === interfaceName) - if (!interfaceDecl) return false - const typeAliasRefs = dependencyAnalyzer.extractTypeAliasReferences( - interfaceDecl, - new Map(localTypeAliases.map((ta) => [ta.getName(), ta])), - ) - return typeAliasRefs.includes(dependency.typeAlias.getName()) - }, - ) - const dependsOnInterface = dependencyAnalysis.typeAliasesDependingOnInterfaces.includes( - dependency.typeAlias.getName(), - ) + // Precompute for O(1) membership lookups + const localTypeAliasesMap = new Map(localTypeAliases.map((ta) => [ta.getName(), ta] as const)) + const interfacesToAliasRefs = new Map<string, Set<string>>() + for (const interfaceName of interfacesDependingOnTypeAliasesSet) { + const interfaceDecl = interfaces.find((i) => i.getName() === interfaceName) + if (!interfaceDecl) continue + const refs = dependencyAnalyzer.extractTypeAliasReferences(interfaceDecl, localTypeAliasesMap) + interfacesToAliasRefs.set(interfaceName, new Set(refs)) + } + orderedDependencies.forEach((dependency) => { + const typeAliasName = dependency.typeAlias.getName() + const interfaceDependsOnThis = Array.from(interfacesToAliasRefs.values()).some((set) => + set.has(typeAliasName), + ) + const dependsOnInterface = typeAliasesDependingOnInterfacesSet.has(typeAliasName)ARCHITECTURE.md (1)
119-123: Grammar: fix truncated “only” and repeated adverbAdjust wording flagged by tooling.
- - Interfaces depending on type aliases only - - Type aliases depending on interfaces only + - Interfaces depending on type aliases + - Type aliases depending on interfacessrc/traverse/dependency-analyzer.ts (2)
53-75: Method doc promises “ordered by their dependencies” but returns an unsorted filterEither implement a deterministic ordering or update the docstring to avoid implying topological sort.
- /** - * Get type aliases that reference interfaces, ordered by their dependencies - */ + /** + * Get type aliases that reference interfaces. + * Note: This does not impose a topological order. + */
124-131: Consider returning Sets for membership-heavy consumersDownstream code performs membership checks; returning
Set<string>s would avoid repeated.includes()calls or immediate conversions. This is optional but simplifies consumers and improves perf.Example:
- ): { - processInterfacesFirst: boolean - typeAliasesDependingOnInterfaces: string[] - interfacesDependingOnTypeAliases: string[] - } { + ): { + processInterfacesFirst: boolean + typeAliasesDependingOnInterfaces: Set<string> + interfacesDependingOnTypeAliases: Set<string> + } { ... - const typeAliasesDependingOnInterfaces: string[] = [] - const interfacesDependingOnTypeAliases: string[] = [] + const typeAliasesDependingOnInterfaces = new Set<string>() + const interfacesDependingOnTypeAliases = new Set<string>() ... - typeAliasesDependingOnInterfaces.push(typeAlias.getName()) + typeAliasesDependingOnInterfaces.add(typeAlias.getName()) ... - interfacesDependingOnTypeAliases.push(interfaceDecl.getName()) + interfacesDependingOnTypeAliases.add(interfaceDecl.getName())If you keep arrays in the public API, consider constructing Sets inside
ts-morph-codegen.ts(as suggested there).
📜 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 (14)
ARCHITECTURE.md(5 hunks)src/handlers/typebox/object/interface-type-handler.ts(1 hunks)src/handlers/typebox/type-reference-handler.ts(2 hunks)src/parsers/parse-interfaces.ts(1 hunks)src/traverse/ast-traversal.ts(1 hunks)src/traverse/dependency-analyzer.ts(1 hunks)src/traverse/dependency-collector.ts(1 hunks)src/traverse/dependency-type.ts(1 hunks)src/ts-morph-codegen.ts(4 hunks)tests/dependency-collector.integration.test.ts(1 hunks)tests/dependency-collector.performance.test.ts(1 hunks)tests/dependency-collector.unit.test.ts(1 hunks)tests/handlers/typebox/interfaces.test.ts(1 hunks)tests/import-resolution.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/handlers/typebox/type-reference-handler.ts (1)
src/utils/typebox-call.ts (1)
getTypeBoxType(9-21)
src/traverse/dependency-analyzer.ts (1)
src/traverse/ast-traversal.ts (1)
ASTTraversal(6-111)
tests/handlers/typebox/interfaces.test.ts (1)
tests/utils.ts (3)
createSourceFile(12-14)generateFormattedCode(26-39)formatWithPrettier(16-24)
src/ts-morph-codegen.ts (2)
src/traverse/dependency-analyzer.ts (1)
DependencyAnalyzer(7-140)src/utils/interface-processing-order.ts (1)
getInterfaceProcessingOrder(3-54)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~119-~119: There might be a mistake here.
Context: ...l Processing**: Handles three scenarios: - Interfaces depending on type aliases onl...
(QB_NEW_EN)
[style] ~121-~121: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... - Type aliases depending on interfaces only - Both dependencies present (three-p...
(ADVERB_REPETITION_PREMIUM)
🔇 Additional comments (13)
tests/dependency-collector.performance.test.ts (1)
1-1: Import path relocation looks goodThe move to the traverse module aligns with the new architecture and keeps tests consuming the public entry under the package namespace.
src/traverse/dependency-collector.ts (1)
4-4: Import path updates are correctSwitching to traverse/* modules keeps related traversal concerns co-located and avoids utils bloat.
Also applies to: 8-8
tests/dependency-collector.integration.test.ts (1)
1-1: Import path move is consistent with the new module layoutGood alignment with traverse/*.
tests/import-resolution.test.ts (1)
1-1: Import path relocation acknowledgedTests now target the traverse module; public API usage remains unchanged.
src/traverse/dependency-type.ts (1)
1-1: Updated TypeDependency import path is correctKeeps type coupling within traverse/*.
tests/dependency-collector.unit.test.ts (1)
1-1: All stale imports removed — no further action neededI ran a sweep across the codebase for any remaining imports of
dependency-collector,dependency-file-resolver, ordependency-typefrom the oldutilspaths (both absolute and deep relative imports) and found no matches. The relocation to thetraversemodule is complete.src/parsers/parse-interfaces.ts (1)
127-141: Ignore the “typeof A” validity concernI’ve verified using TypeScript 5.4.5 that the following compiles without error:
function A<T>(t: T) { return t; } type X<T> = typeof A<T>;Therefore,
typeof A<T>is accepted by the compiler, and the concern about it being illegal in TS 5.4+ is unfounded. The existing use ofts.factory.createTypeQueryNode(exprName, typeArguments)is valid and does not need to be changed.Likely an incorrect or invalid review comment.
src/ts-morph-codegen.ts (2)
96-104: Case comments are clear; control flow matches scenariosThe three-scenario branching reads cleanly and sets up later phases correctly. Good separation of responsibilities.
171-180: Interface processing order gated correctly for Case 1 and Case 3Condition ensures interfaces are parsed after any required type aliases when needed, and skips reprocessing in Case 2. Looks good.
tests/handlers/typebox/interfaces.test.ts (2)
204-233: LGTM: generic types test expectations reflect the new generic factory patternExpected output matches the new “function + Static<ReturnType<…>>” approach and the composite usage in heritage.
235-263: LGTM: generic extension test captures generic heritage correctlyThe expected
B<T>implementation and type aliasing are consistent with the new handlers.ARCHITECTURE.md (1)
78-86: Doc now referencestraverse/dependency-collector.tscorrectly—goodThe module path and capabilities align with the refactor.
src/traverse/dependency-analyzer.ts (1)
137-140: PublicclearCache()passthrough—nice touchProviding an explicit clearing hook is useful for long-lived processes (watch mode).
| The main codegen logic in <mcfile name="ts-morph-codegen.ts" path="src/ts-morph-codegen.ts"></mcfile> implements sophisticated processing order management: | ||
|
|
||
| 1. **Dependency Analysis**: The `getInterfaceProcessingOrder` function analyzes all interfaces to identify inheritance relationships | ||
| 2. **Topological Sorting**: Interfaces are sorted to ensure base interfaces are processed before extended interfaces | ||
| 3. **Circular Dependency Detection**: The algorithm detects and handles circular inheritance scenarios gracefully | ||
| 1. **Dependency Analysis**: Uses `InterfaceTypeDependencyAnalyzer` to analyze complex relationships between interfaces and type aliases | ||
| 2. **Conditional Processing**: Handles three scenarios: | ||
| - Interfaces depending on type aliases only | ||
| - Type aliases depending on interfaces only | ||
| - Both dependencies present (three-phase processing) | ||
| 3. **Topological Sorting**: Ensures types are processed in correct dependency order to prevent "type not found" errors | ||
| 4. **Circular Dependency Detection**: The algorithm detects and handles circular inheritance scenarios gracefully |
There was a problem hiding this comment.
Fix class name: DependencyAnalyzer (not InterfaceTypeDependencyAnalyzer) and clarify scenarios
The documented name doesn’t exist in the codebase and can mislead readers. Also tighten the bullet wording.
-1. **Dependency Analysis**: Uses `InterfaceTypeDependencyAnalyzer` to analyze complex relationships between interfaces and type aliases
-2. **Conditional Processing**: Handles three scenarios:
- - Interfaces depending on type aliases only
- - Type aliases depending on interfaces only
- - Both dependencies present (three-phase processing)
+1. **Dependency Analysis**: Uses `DependencyAnalyzer` to analyze relationships between interfaces and type aliases
+2. **Conditional Processing**: Handles three scenarios:
+ - Interfaces depending on type aliases
+ - Type aliases depending on interfaces
+ - Both dependencies present (three-phase processing)📝 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.
| The main codegen logic in <mcfile name="ts-morph-codegen.ts" path="src/ts-morph-codegen.ts"></mcfile> implements sophisticated processing order management: | |
| 1. **Dependency Analysis**: The `getInterfaceProcessingOrder` function analyzes all interfaces to identify inheritance relationships | |
| 2. **Topological Sorting**: Interfaces are sorted to ensure base interfaces are processed before extended interfaces | |
| 3. **Circular Dependency Detection**: The algorithm detects and handles circular inheritance scenarios gracefully | |
| 1. **Dependency Analysis**: Uses `InterfaceTypeDependencyAnalyzer` to analyze complex relationships between interfaces and type aliases | |
| 2. **Conditional Processing**: Handles three scenarios: | |
| - Interfaces depending on type aliases only | |
| - Type aliases depending on interfaces only | |
| - Both dependencies present (three-phase processing) | |
| 3. **Topological Sorting**: Ensures types are processed in correct dependency order to prevent "type not found" errors | |
| 4. **Circular Dependency Detection**: The algorithm detects and handles circular inheritance scenarios gracefully | |
| The main codegen logic in <mcfile name="ts-morph-codegen.ts" path="src/ts-morph-codegen.ts"></mcfile> implements sophisticated processing order management: | |
| 1. **Dependency Analysis**: Uses `DependencyAnalyzer` to analyze relationships between interfaces and type aliases | |
| 2. **Conditional Processing**: Handles three scenarios: | |
| - Interfaces depending on type aliases | |
| - Type aliases depending on interfaces | |
| - Both dependencies present (three-phase processing) | |
| 3. **Topological Sorting**: Ensures types are processed in correct dependency order to prevent "type not found" errors | |
| 4. **Circular Dependency Detection**: The algorithm detects and handles circular inheritance scenarios gracefully |
🧰 Tools
🪛 LanguageTool
[grammar] ~119-~119: There might be a mistake here.
Context: ...l Processing**: Handles three scenarios: - Interfaces depending on type aliases onl...
(QB_NEW_EN)
[style] ~121-~121: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... - Type aliases depending on interfaces only - Both dependencies present (three-p...
(ADVERB_REPETITION_PREMIUM)
🤖 Prompt for AI Agents
In ARCHITECTURE.md around lines 116 to 124, the doc references a non-existent
class name and vague bullets: change the class reference from
InterfaceTypeDependencyAnalyzer to DependencyAnalyzer, and tighten the three
scenario bullets to concise wording that matches the implementation (interfaces
depend only on aliases; aliases depend only on interfaces; mixed dependencies
requiring phased processing). Also keep mentions of topological sorting and
circular dependency detection but rephrase succinctly to align with actual
behavior in src/ts-morph-codegen.ts.
| // If there are type arguments, create a function call | ||
| if (typeArguments.length > 0) { | ||
| const typeBoxArgs = typeArguments.map((arg) => getTypeBoxType(arg)) | ||
|
|
||
| return ts.factory.createCallExpression( | ||
| ts.factory.createIdentifier(typeName), | ||
| undefined, | ||
| typeBoxArgs, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle qualified names (e.g., ns.Vector) when emitting calls.
The current path only supports identifiers. Qualified names will fall through or be emitted incorrectly if coerced into an Identifier. Build a property access chain from the EntityName for robust handling.
- if (typeArguments.length > 0) {
- const typeBoxArgs = typeArguments.map((arg) => getTypeBoxType(arg))
-
- return ts.factory.createCallExpression(
- ts.factory.createIdentifier(typeName),
- undefined,
- typeBoxArgs,
- )
- }
+ if (typeArguments.length > 0) {
+ const typeBoxArgs = typeArguments.map((arg) => getTypeBoxType(arg))
+ const entity = node.getTypeName()
+ const toExpr = (e: typeof entity): ts.Expression => {
+ const cn = e.compilerNode
+ if (ts.isIdentifier(cn)) return ts.factory.createIdentifier(cn.text)
+ // QualifiedName: recursively build ns.prop.prop...
+ const left = toExpr((e as any).getLeft())
+ const right = (e as any).getRight().getText()
+ return ts.factory.createPropertyAccessExpression(left, right)
+ }
+ return ts.factory.createCallExpression(toExpr(entity as any), undefined, typeBoxArgs)
+ }📝 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 there are type arguments, create a function call | |
| if (typeArguments.length > 0) { | |
| const typeBoxArgs = typeArguments.map((arg) => getTypeBoxType(arg)) | |
| return ts.factory.createCallExpression( | |
| ts.factory.createIdentifier(typeName), | |
| undefined, | |
| typeBoxArgs, | |
| ) | |
| } | |
| // If there are type arguments, create a function call | |
| if (typeArguments.length > 0) { | |
| const typeBoxArgs = typeArguments.map((arg) => getTypeBoxType(arg)) | |
| const entity = node.getTypeName() | |
| const toExpr = (e: typeof entity): ts.Expression => { | |
| const cn = e.compilerNode | |
| if (ts.isIdentifier(cn)) return ts.factory.createIdentifier(cn.text) | |
| // QualifiedName: recursively build ns.prop.prop... | |
| const left = toExpr((e as any).getLeft()) | |
| const right = (e as any).getRight().getText() | |
| return ts.factory.createPropertyAccessExpression(left, right) | |
| } | |
| return ts.factory.createCallExpression(toExpr(entity as any), undefined, typeBoxArgs) | |
| } |
🤖 Prompt for AI Agents
In src/handlers/typebox/type-reference-handler.ts around lines 18 to 27, the
current logic only handles Identifier names when emitting a call for a generic
type and will mis-handle QualifiedName (e.g., ns.Vector<T>); replace the direct
Identifier creation with a helper that converts an EntityName into a nested
PropertyAccessExpression by recursively walking QualifiedName nodes (or using
ts.isIdentifier/ts.isQualifiedName checks) to produce the correct left-hand
expression, then use that expression as the callee for
ts.factory.createCallExpression so calls for both identifiers and qualified
names are emitted correctly.
| const references: string[] = [] | ||
| const visited = new Set<Node>() | ||
|
|
||
| const traverse = (node: Node): void => { | ||
| if (visited.has(node)) return | ||
| visited.add(node) | ||
|
|
||
| // Handle type references | ||
| if (Node.isTypeReference(node)) { | ||
| const typeRefNode = node as TypeReferenceNode | ||
| const typeName = typeRefNode.getTypeName().getText() | ||
|
|
||
| if (interfaces.has(typeName)) { | ||
| references.push(typeName) | ||
| } | ||
| // Continue traversing to handle type arguments in generic instantiations | ||
| } | ||
|
|
||
| // Use forEachChild for better performance | ||
| node.forEachChild(traverse) | ||
| } | ||
|
|
||
| traverse(typeNode) | ||
|
|
||
| // Cache the result | ||
| ASTTraversal.cache.set(cacheKey, references) | ||
| return references | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Deduplicate references and store copies in cache.
Traversals can encounter the same referenced name multiple times (e.g., repeated fields, unions). Use a Set to ensure uniqueness and store/return copies to maintain cache integrity.
- const references: string[] = []
+ const references = new Set<string>()
const visited = new Set<Node>()
@@
- if (Node.isTypeReference(node)) {
+ if (Node.isTypeReference(node)) {
const typeRefNode = node as TypeReferenceNode
const typeName = typeRefNode.getTypeName().getText()
- if (interfaces.has(typeName)) {
- references.push(typeName)
- }
+ if (interfaces.has(typeName)) {
+ references.add(typeName)
+ }
// Continue traversing to handle type arguments in generic instantiations
}
@@
- ASTTraversal.cache.set(cacheKey, references)
- return references
+ const out = Array.from(references)
+ ASTTraversal.cache.set(cacheKey, [...out])
+ return out📝 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 references: string[] = [] | |
| const visited = new Set<Node>() | |
| const traverse = (node: Node): void => { | |
| if (visited.has(node)) return | |
| visited.add(node) | |
| // Handle type references | |
| if (Node.isTypeReference(node)) { | |
| const typeRefNode = node as TypeReferenceNode | |
| const typeName = typeRefNode.getTypeName().getText() | |
| if (interfaces.has(typeName)) { | |
| references.push(typeName) | |
| } | |
| // Continue traversing to handle type arguments in generic instantiations | |
| } | |
| // Use forEachChild for better performance | |
| node.forEachChild(traverse) | |
| } | |
| traverse(typeNode) | |
| // Cache the result | |
| ASTTraversal.cache.set(cacheKey, references) | |
| return references | |
| } | |
| const references = new Set<string>() | |
| const visited = new Set<Node>() | |
| const traverse = (node: Node): void => { | |
| if (visited.has(node)) return | |
| visited.add(node) | |
| // Handle type references | |
| if (Node.isTypeReference(node)) { | |
| const typeRefNode = node as TypeReferenceNode | |
| const typeName = typeRefNode.getTypeName().getText() | |
| if (interfaces.has(typeName)) { | |
| references.add(typeName) | |
| } | |
| // Continue traversing to handle type arguments in generic instantiations | |
| } | |
| // Use forEachChild for better performance | |
| node.forEachChild(traverse) | |
| } | |
| traverse(typeNode) | |
| // Cache the result | |
| const out = Array.from(references) | |
| ASTTraversal.cache.set(cacheKey, [...out]) | |
| return out |
🤖 Prompt for AI Agents
In src/traverse/ast-traversal.ts around lines 23 to 50, the traversal collects
referenced type names into an array that can contain duplicates and the array
instance is stored directly in the cache (allowing external mutation); change
the logic to collect names into a Set to ensure uniqueness during traversal, and
when caching and returning the result convert the Set into a fresh array copy
(e.g., Array.from(set)) so the cached value is an immutable copy and callers
receive their own array instances.
| const traverse = (node: Node): void => { | ||
| if (visited.has(node)) return | ||
| visited.add(node) | ||
|
|
||
| // Handle type references | ||
| if (Node.isTypeReference(node)) { | ||
| const typeRefNode = node as TypeReferenceNode | ||
| const typeName = typeRefNode.getTypeName().getText() | ||
|
|
||
| if (typeAliases.has(typeName)) { | ||
| references.push(typeName) | ||
| } | ||
| return // No need to traverse children of type references | ||
| } |
There was a problem hiding this comment.
Don’t early-return on TypeReference; you’ll miss aliases in type arguments.
In extractTypeAliasReferences, returning immediately after handling a TypeReference prevents traversal into its type arguments. You’ll miss cases like Wrapper.
- if (Node.isTypeReference(node)) {
+ if (Node.isTypeReference(node)) {
const typeRefNode = node as TypeReferenceNode
const typeName = typeRefNode.getTypeName().getText()
if (typeAliases.has(typeName)) {
references.push(typeName)
}
- return // No need to traverse children of type references
+ // Intentionally continue to traverse children to catch nested references
}Also consider switching to a Set here and returning a unique list, mirroring extractInterfaceReferences.
📝 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 traverse = (node: Node): void => { | |
| if (visited.has(node)) return | |
| visited.add(node) | |
| // Handle type references | |
| if (Node.isTypeReference(node)) { | |
| const typeRefNode = node as TypeReferenceNode | |
| const typeName = typeRefNode.getTypeName().getText() | |
| if (typeAliases.has(typeName)) { | |
| references.push(typeName) | |
| } | |
| return // No need to traverse children of type references | |
| } | |
| const traverse = (node: Node): void => { | |
| if (visited.has(node)) return | |
| visited.add(node) | |
| // Handle type references | |
| if (Node.isTypeReference(node)) { | |
| const typeRefNode = node as TypeReferenceNode | |
| const typeName = typeRefNode.getTypeName().getText() | |
| if (typeAliases.has(typeName)) { | |
| references.push(typeName) | |
| } | |
| // Intentionally continue to traverse children to catch nested references | |
| } | |
| // Continue traversal into child nodes | |
| node.forEachChild(child => traverse(child)) | |
| } |
🤖 Prompt for AI Agents
In src/traverse/ast-traversal.ts around lines 66 to 79, the traversal currently
returns immediately after handling a TypeReference which causes type arguments
(e.g., Wrapper<AliasX>) to be skipped and their aliases missed; change the logic
to not early-return for TypeReference nodes — record the referenced typeName if
it matches a typeAlias and then continue traversing the node's children
(including type arguments) so nested aliases are discovered; also change
references from an array to a Set (or deduplicate before returning) so the
result mirrors extractInterfaceReferences and only returns unique alias names.
| // Check interfaces that depend on type aliases | ||
| for (const interfaceDecl of interfaces) { | ||
| const typeAliasRefs = this.extractTypeAliasReferences(interfaceDecl, typeAliasMap) | ||
| if (typeAliasRefs.length > 0) { | ||
| interfacesDependingOnTypeAliases.push(interfaceDecl.getName()) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Incomplete interface→type-alias reference extraction misses interface members
extractTypeAliasReferences (delegated to ASTTraversal) only inspects type parameter constraints and heritage clauses. It ignores interface member types (properties, methods, index signatures), which is the most common place an interface references a local type alias. This can produce incorrect ordering decisions and out-of-order generation.
Follow-up: extend ASTTraversal.extractTypeAliasReferences to traverse interface members. Example implementation:
// In src/traverse/ast-traversal.ts
static extractTypeAliasReferences(
interfaceDecl: InterfaceDeclaration,
typeAliases: Map<string, TypeAliasDeclaration>,
): string[] {
const cacheKey = `type_alias_refs_${interfaceDecl.getName()}_${interfaceDecl.getText()}`
const cached = ASTTraversal.cache.get(cacheKey)
if (cached) return cached
const references = new Set<string>()
const visit = (node: Node): void => {
if (Node.isTypeReference(node)) {
const typeName = (node as TypeReferenceNode).getTypeName().getText()
if (typeAliases.has(typeName)) references.add(typeName)
}
node.forEachChild(visit)
}
// 1) Type parameters (constraints)
for (const tp of interfaceDecl.getTypeParameters()) {
const c = tp.getConstraint()
if (c) visit(c)
}
// 2) Heritage clauses
for (const hc of interfaceDecl.getHeritageClauses()) {
for (const t of hc.getTypeNodes()) visit(t)
}
// 3) Members: properties, methods, index signatures, call signatures
for (const m of interfaceDecl.getMembers()) {
if (Node.isPropertySignature(m) || Node.isMethodSignature(m) || Node.isIndexSignatureDeclaration(m)) {
const t = m.getTypeNode()
if (t) visit(t)
}
// Also inspect method parameters/return types
if (Node.isMethodSignature(m)) {
for (const p of m.getParameters()) {
const pt = p.getTypeNode()
if (pt) visit(pt)
}
const rt = m.getReturnTypeNode()
if (rt) visit(rt)
}
}
const result = Array.from(references)
ASTTraversal.cache.set(cacheKey, result)
return result
}Additionally, consider de-duplicating reference arrays returned by traversal to avoid repeated work downstream.
🤖 Prompt for AI Agents
In src/traverse/dependency-analyzer.ts around lines 111-118, the current call to
extractTypeAliasReferences misses type-alias uses inside interface members
(properties, methods, index signatures, etc.), causing incorrect dependency
ordering; update ASTTraversal.extractTypeAliasReferences to recursively traverse
interface member nodes: visit type parameter constraints, heritage clauses, and
every member's type node (properties, index signatures, call signatures), and
for method signatures also visit each parameter type and the return type, using
a visitor that collects TypeReference nodes by name, deduplicate results, cache
by a stable key, and return an array of referenced type-alias names.
| const dependencyAnalyzer = new DependencyAnalyzer() | ||
|
|
There was a problem hiding this comment.
Clear analysis caches per invocation to avoid stale results/memory growth
ASTTraversal uses a static cache. Instantiate-and-forget without clearing can leak across runs and produce stale dependency sets when generateCode is called multiple times within the same process.
Apply this diff to clear before use and after generation:
- const dependencyAnalyzer = new DependencyAnalyzer()
+ const dependencyAnalyzer = new DependencyAnalyzer()
+ // Ensure clean analysis state for this invocation
+ dependencyAnalyzer.clearCache()And replace the return at the end:
- return newSourceFile.getFullText()
+ const result = newSourceFile.getFullText()
+ // Best-effort cleanup
+ dependencyAnalyzer.clearCache()
+ return result📝 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 dependencyAnalyzer = new DependencyAnalyzer() | |
| const dependencyAnalyzer = new DependencyAnalyzer() | |
| // Ensure clean analysis state for this invocation | |
| dependencyAnalyzer.clearCache() | |
| // ... existing code that uses dependencyAnalyzer ... | |
| const result = newSourceFile.getFullText() | |
| // Best‐effort cleanup | |
| dependencyAnalyzer.clearCache() | |
| return result |
🤖 Prompt for AI Agents
In src/ts-morph-codegen.ts around lines 72-73, ASTTraversal uses a static cache
so you must clear its cache before creating DependencyAnalyzer and again after
generateCode finishes; call the ASTTraversal cache-clear helper (e.g.,
ASTTraversal.clearCache()) immediately before instantiating new
DependencyAnalyzer and also ensure you clear it just before the function returns
(replace the current return with a clear-cache call followed by the original
return), to prevent stale results and memory growth across multiple generateCode
invocations.
| // Process remaining type aliases that depend on interfaces (Case 2 and Case 3) | ||
| if (hasTypeAliasesDependingOnInterfaces) { | ||
| orderedDependencies.forEach((dependency) => { | ||
| const dependsOnInterface = dependencyAnalysis.typeAliasesDependingOnInterfaces.includes( | ||
| dependency.typeAlias.getName(), | ||
| ) | ||
| if (dependsOnInterface && !processedTypes.has(dependency.typeAlias.getName())) { | ||
| typeAliasParser.parseWithImportFlag(dependency.typeAlias, dependency.isImported) | ||
| } | ||
| }) | ||
|
|
||
| // Process any remaining local types that weren't included in the dependency graph | ||
| if (exportEverything) { | ||
| localTypeAliases.forEach((typeAlias) => { | ||
| if (!processedTypes.has(typeAlias.getName())) { | ||
| typeAliasParser.parseWithImportFlag(typeAlias, false) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential unresolved cycles between a type alias and interface
If an interface references a type alias and that type alias also references any interface (mutual cross-edge), neither side is generated before the other with the current phases. That can surface as missing identifier errors during codegen. Consider explicit cycle detection across the bipartite graph (interfaces ↔ type aliases) and either:
- fall back to a single-phase emit for the cycle participants, or
- emit stubs/forward refs for one side, or
- surface a clear diagnostic pointing to the cycle.
I can provide a minimal detection implementation if you want it integrated into DependencyAnalyzer.
🏁 Script executed:
#!/bin/bash
set -e
# Show the code around the type alias processing snippet
rg -n --context 5 'Process remaining type aliases that depend on interfaces' -g 'src/ts-morph-codegen.ts'
# Locate the DependencyAnalyzer class to inspect its logic
rg -n 'class DependencyAnalyzer' -g 'src/**/*.ts'
# Search for any cycle detection or handling in the codebase
rg -n 'cycle' -g 'src'
# Search for topological sort or similar dependency ordering logic
rg -n 'topologicalSort' -g 'src'Length of output: 1132
🏁 Script executed:
#!/bin/bash
set -e
# Locate the dependency-analyzer file
file=$(fd dependency-analyzer.ts | head -n1)
echo "Using file: $file"
# Show first 200 lines of the file
sed -n '1,200p' "$file"
# Find typeAliasesDependingOnInterfaces property in the file
rg -n 'typeAliasesDependingOnInterfaces' "$file"
# Check for any mention of 'cycle' in src
rg -n 'cycle' src
# Check for sort or ordering logic in the file
rg -n 'sort' "$file"Length of output: 5091
Add explicit cycle detection between interfaces and type aliases
The current DependencyAnalyzer (in src/traverse/dependency-analyzer.ts) only collects two flat lists—typeAliasesDependingOnInterfaces and interfacesDependingOnTypeAliases—and flags whether to process interfaces first. There is no logic to detect or break dependency cycles, so if a type alias A references interface I and I in turn references type alias A (or any longer mutual chain), neither will be emitted before its dependency, leading to missing-symbol errors at codegen time.
To address this, please extend DependencyAnalyzer to:
- Detect cycles in the bipartite graph of interfaces ↔ type aliases.
- For any strongly-connected component (cycle), choose one of the following strategies:
- Emit both participants in a single combined phase (i.e. skip ordering within the cycle).
- Generate forward-reference stubs for one side before emitting the other.
- Throw a clear, user-facing diagnostic listing the cycle participants.
Suggested next steps:
- In
DependencyAnalyzer, add adetectCycles()method that builds a directed graph (nodes = all interface and type-alias names; edges = “A → B if A references B”) and finds strongly-connected components (e.g. via Tarjan’s algorithm). - Modify
analyzeProcessingOrderto calldetectCycles(), then:- Remove intra-cycle edges when computing the topo sort for acyclic portions.
- Handle each cycle component by emitting stubs or emitting all members together.
- Surface a warning or error if a cycle cannot be safely broken automatically.
Let me know if you’d like a minimal cycle-detection implementation to integrate directly into DependencyAnalyzer.
🤖 Prompt for AI Agents
In src/ts-morph-codegen.ts around lines 183 to 202, the current processing
assumes no cycles between interfaces and type aliases which can cause
missing-symbol errors when mutual references exist; update the
DependencyAnalyzer to detect and handle cycles: add a detectCycles() method that
builds a directed graph of nodes (interface and type alias names) with edges A→B
when A references B and use Tarjan (or similar) to find strongly connected
components (SCCs); modify analyzeProcessingOrder to call detectCycles(), remove
or ignore intra-SCC edges for the topological sort of the acyclic portion, and
for each SCC either (a) emit all members of the SCC together (combined phase),
(b) emit forward-reference stubs for one side before the other, or (c) surface a
clear diagnostic listing the cycle participants if it cannot be auto-broken;
ensure detectCycles returns SCCs and hooks into existing flags
(typeAliasesDependingOnInterfaces / interfacesDependingOnTypeAliases) and add
user-facing warnings/errors for unresolvable cycles.
| test('generic types with extended type', () => { | ||
| const sourceFile = createSourceFile( | ||
| project, | ||
| ` | ||
| declare const A: readonly ["a", "b"] | ||
| type A = typeof A[number] | ||
| interface B<T extends A> { a: T } | ||
| type C = B<'a'> | ||
| type D = B<'b'> | ||
| `, | ||
| ) | ||
|
|
||
| type A<T extends TSchema> = Static<ReturnType<typeof A<T>>>; | ||
| expect(generateFormattedCode(sourceFile)).resolves.toBe( | ||
| formatWithPrettier( | ||
| ` | ||
| const A = Type.Union([Type.Literal('a'), Type.Literal('b')]) | ||
| const B = Type.Composite([A(Type.Number()), Type.Object({ | ||
| b: Type.Number() | ||
| })]); | ||
| type A = Static<typeof A> | ||
| type B = Static<typeof B>; | ||
| `, | ||
| true, | ||
| true, | ||
| ), | ||
| ) | ||
| const B = <T extends TSchema>(T: T) => Type.Object({ | ||
| a: T | ||
| }) | ||
| type B<T extends TSchema> = Static<ReturnType<typeof B<T>>> | ||
| const C = B(Type.Literal('a')) | ||
| type C = Static<typeof C> | ||
| const D = B(Type.Literal('b')) | ||
| type D = Static<typeof D> | ||
| `, | ||
| true, | ||
| true, | ||
| ), | ||
| ) | ||
| }) |
There was a problem hiding this comment.
Inconsistent formatting (missing semicolons) will likely fail against Prettier snapshot
Earlier expectations in this file use semicolons; this block omits them. Given formatWithPrettier is applied with the same options, the mismatch will cause failures.
Apply this diff to normalize formatting:
- const A = Type.Union([Type.Literal('a'), Type.Literal('b')])
+ const A = Type.Union([Type.Literal('a'), Type.Literal('b')]);
- type A = Static<typeof A>
+ type A = Static<typeof A>;
const B = <T extends TSchema>(T: T) => Type.Object({
a: T
- })
+ });
- type B<T extends TSchema> = Static<ReturnType<typeof B<T>>>
+ type B<T extends TSchema> = Static<ReturnType<typeof B<T>>>;
- const C = B(Type.Literal('a'))
+ const C = B(Type.Literal('a'));
- type C = Static<typeof C>
+ type C = Static<typeof C>;
- const D = B(Type.Literal('b'))
+ const D = B(Type.Literal('b'));
- type D = Static<typeof D>
+ type D = Static<typeof D>;Also consider adding a regression test for mutual interface/type-alias dependency to validate the new multi-phase ordering.
📝 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.
| test('generic types with extended type', () => { | |
| const sourceFile = createSourceFile( | |
| project, | |
| ` | |
| declare const A: readonly ["a", "b"] | |
| type A = typeof A[number] | |
| interface B<T extends A> { a: T } | |
| type C = B<'a'> | |
| type D = B<'b'> | |
| `, | |
| ) | |
| type A<T extends TSchema> = Static<ReturnType<typeof A<T>>>; | |
| expect(generateFormattedCode(sourceFile)).resolves.toBe( | |
| formatWithPrettier( | |
| ` | |
| const A = Type.Union([Type.Literal('a'), Type.Literal('b')]) | |
| const B = Type.Composite([A(Type.Number()), Type.Object({ | |
| b: Type.Number() | |
| })]); | |
| type A = Static<typeof A> | |
| type B = Static<typeof B>; | |
| `, | |
| true, | |
| true, | |
| ), | |
| ) | |
| const B = <T extends TSchema>(T: T) => Type.Object({ | |
| a: T | |
| }) | |
| type B<T extends TSchema> = Static<ReturnType<typeof B<T>>> | |
| const C = B(Type.Literal('a')) | |
| type C = Static<typeof C> | |
| const D = B(Type.Literal('b')) | |
| type D = Static<typeof D> | |
| `, | |
| true, | |
| true, | |
| ), | |
| ) | |
| }) | |
| test('generic types with extended type', () => { | |
| const sourceFile = createSourceFile( | |
| project, | |
| ` | |
| declare const A: readonly ["a", "b"] | |
| type A = typeof A[number] | |
| interface B<T extends A> { a: T } | |
| type C = B<'a'> | |
| type D = B<'b'> | |
| `, | |
| ) | |
| expect(generateFormattedCode(sourceFile)).resolves.toBe( | |
| formatWithPrettier( | |
| ` | |
| const A = Type.Union([Type.Literal('a'), Type.Literal('b')]); | |
| type A = Static<typeof A>; | |
| const B = <T extends TSchema>(T: T) => Type.Object({ | |
| a: T | |
| }); | |
| type B<T extends TSchema> = Static<ReturnType<typeof B<T>>>; | |
| const C = B(Type.Literal('a')); | |
| type C = Static<typeof C>; | |
| const D = B(Type.Literal('b')); | |
| type D = Static<typeof D>; | |
| `, | |
| true, | |
| true, | |
| ), | |
| ) | |
| }) |
🤖 Prompt for AI Agents
In tests/handlers/typebox/interfaces.test.ts around lines 265 to 302, the
expected formatted code block is missing semicolons which creates inconsistent
formatting with the rest of the file and will fail the Prettier snapshot; update
the expected string to include semicolons at the end of statements (e.g., after
Type.Union(...) declarations, type declarations, const B = ... arrow function
return, const C/D assignments, etc.) to match the file's established style and
Prettier options, and add a small regression test case that covers a mutual
interface/type-alias dependency to ensure multi-phase ordering is preserved
(place it near related tests and follow existing test patterns and formatting).
Improve the type dependency analysis by:
DependencyAnalyzerclass to handle the analysisThis ensures that the code generation process can handle complex type dependencies
correctly, allowing for more robust and reliable schema generation.