feat(dependency-traversal): Implement comprehensive type dependency traversal #12
Conversation
…raversal This commit introduces a comprehensive type dependency traversal implementation that combines AST traversal, dependency collection, and analysis. The key changes are: - Implement `DependencyTraversal` class that manages the dependency graph using Graphology library - Add methods to extract interface and type alias references from type declarations - Introduce `TypeReferenceExtractor` interface to allow customization of type reference extraction - Implement `ProcessingOrderResult` to analyze the optimal processing order for interfaces and type aliases These changes enable efficient management of type dependencies, which is crucial for generating code that respects the correct order of type declarations.
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces the Map-based DependencyCollector/Analyzer/ASTTraversal with a Graphology-backed DependencyTraversal. Updates codegen to use the new traversal phases and methods, adjusts package dependencies, removes old traversal utilities, and updates tests (removals, new traversal unit tests, revised expectations, commented scaffolds). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Codegen as ts-morph-codegen
participant Traversal as DependencyTraversal
participant Resolver as FileResolver
participant Graph as Graphology DirectedGraph
User->>Codegen: generate(project, sourceFile, options)
Codegen->>Traversal: new DependencyTraversal(...)
Codegen->>Traversal: addLocalTypes(localTypeAliases, sourceFile)
Codegen->>Traversal: collectFromImports(importDeclarations)
Traversal->>Resolver: resolve(moduleSpecifier)
Resolver-->>Traversal: SourceFile | undefined
Traversal->>Graph: add nodes/edges (types, refs)
Codegen->>Traversal: analyzeProcessingOrder(typeAliases, interfaces)
Codegen->>Traversal: filterUnusedImports() [if !exportEverything]
Codegen->>Traversal: getTopologicallySortedTypes(exportEverything)
Traversal->>Graph: topological sort (graphology-dag)
Traversal-->>Codegen: ordered TypeDependency[]
Codegen-->>User: emit types in sorted order
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ARCHITECTURE.md (1)
369-397: Outdated references to “DependencyCollector” and related tests.This section documents “DependencyCollector Optimizations” and mentions
tests/dependency-collector.performance.test.ts, which conflict with the Graphology-only architecture and removed modules.Proposed clean-up:
- Replace “DependencyCollector Optimizations” with “DependencyTraversal Optimizations” or remove if redundant.
- Update/remove performance test references to match current files, or add an equivalent
dependency-traversal.performance.test.ts.I can draft the updated performance section once you confirm the current test filenames.
tests/traverse/dependency-collector.integration.test.ts (1)
1-430: Port or remove lingering DependencyCollector referencesThe
rgsweep shows that remnants of the deletedDependencyCollectorAPI still exist in the codebase:
- tests/traverse/dependency-collector.integration.test.ts
- tests/traverse/dependency-collector.performance.test.ts
- ARCHITECTURE.md (sections “DependencyCollector” and “DependencyCollector Optimizations”)
These files currently target an API that no longer exists, which adds noise and will break CI. You should either fully migrate these to the new
DependencyTraversalAPI or delete them until you’re ready to reintroduce traversal tests.Action items:
- In each test file:
- Replace imports of
DependencyCollectorwithDependencyTraversal.- Update instantiation:
const traversal = new DependencyTraversal().- Swap out calls to
collector.collectFromImports(...)and cache-clearing methods for the equivalent traversal methods (e.g.traversal.collectFromImports(...)+traversal.getTopologicallySortedTypes()and any traversal reset calls).- Remove or adapt any now-obsolete cache-clear calls (
DependencyCollector.clearGlobalCache()).- In ARCHITECTURE.md:
- Update the “DependencyCollector” headings and links to refer to
DependencyTraversal, or remove those sections if documentation isn’t yet available.- After refactoring, re-run:
to confirm no stray references remain.rg -n -e '\bDependencyCollector\b' -e 'dependency-collector'
🧹 Nitpick comments (21)
tests/traverse/dependency-collector.performance.test.ts (7)
1-341: Replace dead, legacy scaffold with a skipped, minimal DependencyTraversal perf suite (or delete the file).All code is commented out and targets the removed DependencyCollector, yielding zero coverage and future drift. Convert to a small, skipped scaffold referencing DependencyTraversal so the intent is preserved without flakiness, or remove the file entirely.
Apply this diff to replace the commented content with a gated, skipped scaffold aligned to the new API:
-// import { DependencyCollector } from '@daxserver/validation-schema-codegen/traverse/dependency-collector' -// import { createSourceFile } from '@test-fixtures/utils' -// import { beforeEach, describe, expect, test } from 'bun:test' -// import { Project } from 'ts-morph' -// -// describe('DependencyCollector Performance Tests', () => { -// let project: Project -// let collector: DependencyCollector -// -// beforeEach(() => { -// project = new Project() -// collector = new DependencyCollector() -// }) -// -// ... (all commented tests) -// }) +import { beforeEach, describe, test } from 'bun:test' +import { Project } from 'ts-morph' +// Note: wire this when DependencyTraversal API is finalized in this PR +// import { DependencyTraversal } from '@daxserver/validation-schema-codegen/traverse/dependency-traversal' +// import { createSourceFile } from '@test-fixtures/utils' + +// Gate perf on env and keep it skipped by default to avoid CI flakiness. +const PERF_ENABLED = process.env.RUN_PERF_TESTS === '1' +const maybeDescribe = PERF_ENABLED ? describe : describe.skip + +maybeDescribe('DependencyTraversal performance (ported from legacy collector) [PERF]', () => { + let project: Project + // let traversal: DependencyTraversal + + beforeEach(() => { + project = new Project() + // traversal = new DependencyTraversal(/* extractor/config */) + }) + + test.todo('deep import chains (50 files)') + test.todo('wide import trees (100 imports)') + test.todo('cache key overhead (20 complex types x5 passes)') + test.todo('memory with large cache (200 files)') + test.todo('diamond pattern revisits (base/left/right + 30 leaves)') + test.todo('complex topological sort (web of 50 types)') + test.todo('large type definitions (50 x 100 fields)') + test.todo('graph operations stress (100 nodes)') +})
6-14: Don’t keep commented describe/beforeEach; either skip or gate.Use describe.skip or an env gate to prevent accidental activation and CI flakiness while keeping tests visible to maintainers.
15-70: Prefer benchmarks over timing assertions in unit tests.Hard time thresholds are brittle across CI. If you need performance tracking, move these to Bun benchmarks.
Example benchmark (new file suggestion, not a direct diff here):
import { bench, run } from 'mitata' // or Bun's built-in bench when available bench('deep import chain (50)', () => { // setup synthetic files, run traversal, assert counts outside timing }) await run()
72-168: Cache-oriented tests need new measurement points.The graphology-backed design won’t mirror “collector cache” behavior. Measure graph node/edge counts and traversal order size instead of Map/cache timings.
170-221: Diamond dependency: validate single-visit semantics via traversal order, not import scans.With the graph, assert that base is not redundantly processed by checking the processing order or visit counts exposed by ProcessingOrderResult.
222-263: Complex topo sort: assert on ProcessingOrderResult invariants.Replace dependencies.length checks with:
- All edges go from earlier to later in the order.
- Order contains expected subset/superset of nodes.
265-341: Memory-heavy scenarios: keep opt-in and small by default.Generating 50x100-field types and 200-file graphs can destabilize CI. Keep behind RUN_PERF_TESTS=1 and consider smaller defaults with a larger locally-run variant.
Optional gating pattern (additive to the main diff if you keep individual tests active later):
if (process.env.RUN_PERF_TESTS !== '1') test.skip('large type definitions (50 x 100 fields)', () => {})package.json (1)
36-41: Consider pinning Graphology versions for determinism.Caret ranges are fine; if traversal order determinism matters across CI nodes, pinning exact versions reduces flakiness from transitive updates.
ARCHITECTURE.md (3)
118-151: Remove or clearly scope legacy “Collector”/multi-mechanism references.This section claims a single, unified Graphology mechanism, but later the doc still references multiple trackers and caches outside the graph model. Clarify that any caches (e.g., on nodes) are graph-backed, or drop references that imply parallel systems.
160-166: Confirm Interface inheritance uses Type.Composite in code.Doc states
InterfaceTypeHandleremitsType.Composite([Base, Type.Object(...)]). Verify implementation and update examples if it still usesType.Intersect.If using Composite, add a short example snippet to this section to cement expected output.
322-341: Minor: tighten repetition in Graphology optimization bullets.These bullets repeat earlier claims. Consider consolidating to reduce length and improve scanability.
tests/traverse/dependency-ordering.test.ts (1)
1-93: Avoid committed fully-commented test files; either enable or drop.As-is, this adds no coverage. Prefer enabling at least the first test, or delete until ready to run.
Apply this minimal diff to enable the first test and keep the second as a TODO:
-// import { formatWithPrettier, generateFormattedCode } from '@test-fixtures/utils' -// import { beforeEach, describe, expect, test } from 'bun:test' -// import { Project } from 'ts-morph' +import { formatWithPrettier, generateFormattedCode } from '@test-fixtures/utils' +import { beforeEach, describe, expect, test } from 'bun:test' +import { Project } from 'ts-morph' -// describe('Dependency Ordering Bug', () => { -// let project: Project +describe('Dependency Ordering', () => { + let project: Project -// beforeEach(() => { -// project = new Project() -// }) + beforeEach(() => { + project = new Project() + }) -// test('should define StringSnakDataValue before using it', () => { + test('should define StringSnakDataValue before using it', () => { -// const sourceFile = project.createSourceFile( + const sourceFile = project.createSourceFile( -// 'test.ts', -// ` + 'test.ts', + ` -// export type CommonsMediaSnakDataValue = StringSnakDataValue; -// export type StringSnakDataValue = { + export type CommonsMediaSnakDataValue = StringSnakDataValue; + export type StringSnakDataValue = { -// value: string; -// type: 'string'; -// }; -// `, -// ) + value: string; + type: 'string'; + }; + `, + ) -// expect(generateFormattedCode(sourceFile, true)).resolves.toBe( + expect(generateFormattedCode(sourceFile, true)).resolves.toBe( -// formatWithPrettier(` + formatWithPrettier(` -// export const StringSnakDataValue = Type.Object({ + export const StringSnakDataValue = Type.Object({ -// value: Type.String(), -// type: Type.Literal('string'), -// }); + value: Type.String(), + type: Type.Literal('string'), + }); -// export type StringSnakDataValue = Static<typeof StringSnakDataValue>; + export type StringSnakDataValue = Static<typeof StringSnakDataValue>; -// export const CommonsMediaSnakDataValue = StringSnakDataValue; + export const CommonsMediaSnakDataValue = StringSnakDataValue; -// export type CommonsMediaSnakDataValue = Static<typeof CommonsMediaSnakDataValue>; -// `), -// ) -// }) + export type CommonsMediaSnakDataValue = Static<typeof CommonsMediaSnakDataValue>; + `), + ) + }) -// test('should handle complex dependency chains correctly', () => { + // test('should handle complex dependency chains correctly', () => { + // // TODO: enable after traversal ordering is finalized + // }) -// }) +})tests/traverse/dependency-collector.unit.test.ts (3)
1-5: Tighten type-only imports
DefaultFileResolveris only used for a type-cast; prefer importing justFileResolverand cast to that.-import type { - DefaultFileResolver, - FileResolver, -} from '@daxserver/validation-schema-codegen/traverse/dependency-file-resolver' +import type { + FileResolver, +} from '@daxserver/validation-schema-codegen/traverse/dependency-file-resolver'
34-37: Cast to the interface, not the concrete classAvoid coupling the test to the default implementation; cast to
FileResolver.- const mockFileResolver = {} as DefaultFileResolver + const mockFileResolver = {} as FileResolver
162-177: Test name misleads (no topo assertions made)This test doesn’t validate topological ordering—only collection. Either rename or add assertions using real
ts-morphnodes in a future test.- test('should handle dependency resolution', () => { + test('should add local types without errors', () => {src/ts-morph-codegen.ts (3)
113-116: Remove duplicate unused-import filtering
getTopologicallySortedTypes(false)already callsfilterUnusedImports(). The extra call is redundant.- // Filter unused imports if exportEverything is false - if (!exportEverything) { - dependencyTraversal.filterUnusedImports() - }
117-118: Guard against cycles in type aliasesTopological sort will throw on cycles. Add a safe fallback with a clear error or degrade gracefully.
- const orderedDependencies = dependencyTraversal.getTopologicallySortedTypes(exportEverything) + let orderedDependencies: ReturnType<typeof dependencyTraversal.getTopologicallySortedTypes> + try { + orderedDependencies = dependencyTraversal.getTopologicallySortedTypes(exportEverything) + } catch (err) { + // Fallback: emit in insertion order; consider surfacing a diagnostic to the caller. + orderedDependencies = [...dependencyTraversal.getDependencies().values()] + }
129-141: Avoid remapping type aliases inside the loopPrecompute the map once to reduce allocations.
- orderedDependencies.forEach((dependency) => { + const localTypeAliasMap = new Map(localTypeAliases.map((ta) => [ta.getName(), ta])) + orderedDependencies.forEach((dependency) => { const interfaceDependsOnThis = dependencyAnalysis.interfacesDependingOnTypeAliases.some( (interfaceName) => { const interfaceDecl = interfaces.find((i) => i.getName() === interfaceName) if (!interfaceDecl) return false - const typeAliasRefs = dependencyTraversal.extractTypeAliasReferences( - interfaceDecl, - new Map(localTypeAliases.map((ta) => [ta.getName(), ta])), - ) + const typeAliasRefs = dependencyTraversal.extractTypeAliasReferences( + interfaceDecl, + localTypeAliasMap, + ) return typeAliasRefs.includes(dependency.typeAlias.getName()) }, )src/traverse/dependency-traversal.ts (3)
399-414: Topological sort can throw on cycles; handle gracefullyProvide a safer sort to avoid crashing on cyclic type aliases.
- const sortedNodes = topologicalSort(this.dependencyGraph) + let sortedNodes: string[] + try { + sortedNodes = topologicalSort(this.dependencyGraph) + } catch (err) { + // Fallback: maintain current node order; optionally expose diagnostics upstream. + sortedNodes = (this.dependencyGraph.nodes() as string[]) ?? [] + }
155-182: Duplicate type-reference traversal logic
extractTypeReferencesduplicatesDefaultTypeReferenceExtractor.extractTypeReferences. Consider delegating to the injected extractor for consistency and single source of truth.
204-245: Potential node-id collisions between file paths and type namesFiles use raw
filePathas node ids; a type alias named like a path could collide. Consider namespacing ids (e.g.,file:/...vstype:Name) to harden the graph.
📜 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 ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
ARCHITECTURE.md(4 hunks)package.json(2 hunks)src/traverse/ast-traversal.ts(0 hunks)src/traverse/dependency-analyzer.ts(0 hunks)src/traverse/dependency-collector.ts(0 hunks)src/traverse/dependency-traversal.ts(1 hunks)src/traverse/dependency-type.ts(0 hunks)src/ts-morph-codegen.ts(5 hunks)tests/dependency-collector.integration.test.ts(0 hunks)tests/dependency-collector.performance.test.ts(0 hunks)tests/dependency-ordering.test.ts(0 hunks)tests/export-everything.test.ts(1 hunks)tests/import-resolution.test.ts(0 hunks)tests/traverse/dependency-collector.integration.test.ts(1 hunks)tests/traverse/dependency-collector.performance.test.ts(1 hunks)tests/traverse/dependency-collector.unit.test.ts(9 hunks)tests/traverse/dependency-ordering.test.ts(1 hunks)
💤 Files with no reviewable changes (8)
- tests/dependency-collector.performance.test.ts
- src/traverse/dependency-analyzer.ts
- tests/dependency-ordering.test.ts
- tests/import-resolution.test.ts
- src/traverse/ast-traversal.ts
- tests/dependency-collector.integration.test.ts
- src/traverse/dependency-type.ts
- src/traverse/dependency-collector.ts
🧰 Additional context used
🧬 Code graph analysis (3)
src/traverse/dependency-traversal.ts (1)
src/traverse/dependency-file-resolver.ts (2)
FileResolver(3-8)DefaultFileResolver(10-26)
tests/traverse/dependency-collector.unit.test.ts (2)
src/traverse/dependency-traversal.ts (1)
DependencyTraversal(37-605)src/traverse/dependency-file-resolver.ts (1)
DefaultFileResolver(10-26)
src/ts-morph-codegen.ts (1)
src/traverse/dependency-traversal.ts (1)
DependencyTraversal(37-605)
🪛 LanguageTool
ARCHITECTURE.md
[grammar] ~107-~107: There might be a mistake here.
Context: ...y ordering with export-aware preferences - Uses graphology-dag for circular depende...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...ndency detection and topological sorting - Handles complex dependency scenarios inc...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...es code with appropriate export handling - Types are processed sequentially in the ...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...a structure for dependency relationships - Topological Sorting: Employs `topologi...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ...ering with circular dependency detection - Preference-Based Ordering: Implements ...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: ...erence()` for export-aware type ordering - Memory Efficiency: Direct Graphology u...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...y management for large dependency graphs - Type Safety: Full TypeScript support t...
(QB_NEW_EN)
[grammar] ~328-~328: There might be a mistake here.
Context: ...ted graph data structures and algorithms - Optimized Performance: Benefits from G...
(QB_NEW_EN)
[grammar] ~329-~329: There might be a mistake here.
Context: ...nal implementations for graph operations - Advanced Graph Algorithms: Direct acce...
(QB_NEW_EN)
[grammar] ~330-~330: There might be a mistake here.
Context: ...m (graphology-dag, graphology-traversal) - Type Safety: Full TypeScript support t...
(QB_NEW_EN)
[grammar] ~331-~331: There might be a mistake here.
Context: ...support through graphology-types package - Memory Efficiency: Graphology's optimi...
(QB_NEW_EN)
[grammar] ~332-~332: There might be a mistake here.
Context: ...mized memory management for large graphs - Unified Architecture: Single module el...
(QB_NEW_EN)
🔇 Additional comments (6)
tests/traverse/dependency-collector.performance.test.ts (1)
1-5: Update imports to useDependencyTraversaland existing test utilitiesThe
DependencyCollectorimport has been replaced by the newDependencyTraversalclass (exported insrc/traverse/dependency-traversal.ts), and the test fixtures under@test-fixtures/utilsare already in use elsewhere. Please update the commented imports intests/traverse/dependency-collector.performance.test.ts(and similarly in the integration test) to the following:import { DependencyTraversal } from '@daxserver/validation-schema-codegen/traverse/dependency-traversal' import { createSourceFile } from '@test-fixtures/utils' import { beforeEach, describe, expect, test } from 'bun:test' import { Project } from 'ts-morph'package.json (1)
18-18: Add: graphology-types devDependency — looks good.Type definitions in devDependencies are appropriate. No issues.
tests/export-everything.test.ts (1)
80-82: Reordering expectation updated — OK.This matches a topo order that prefers imported types first. Ensure the traversal keeps a stable tie-break to avoid nondeterministic reorders across runs.
If not already, sort adjacency lists (or node IDs) before topological sorting to guarantee stable output when multiple valid orders exist.
ARCHITECTURE.md (2)
78-89: Good: clear “DependencyTraversal” overview.Section accurately explains the Graphology-centric approach and export-aware ordering.
99-117: Fix bullet/formatting glitches in the three-phase description.Several bullets render on the same line (e.g., “ordering with export-aware preferences - Uses graphology-dag ...”). Insert proper newlines so each bullet is separate.
Apply consistent Markdown list formatting; example:
- - Topological sorting with export-aware preferences - Uses graphology-dag ... + - Topological sorting with export-aware preferences + - Uses graphology-dag for circular dependency detection and topological sortingLikely an incorrect or invalid review comment.
tests/traverse/dependency-collector.unit.test.ts (1)
140-158: Circular import guard looks solidNice use of the resolver/mocked recursion to assert visit-short-circuiting.
| "type": "module", | ||
| "dependencies": { | ||
| "graphology": "^0.26.0", | ||
| "graphology-dag": "^0.4.1", | ||
| "graphology-traversal": "^0.3.1" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Graphology ESM/CJS interop under "type": "module".
Graphology packages have had mixed ESM/CJS stories. With "type": "module", confirm imports like import Graph from 'graphology' work in Bun/Node for your test/runtime environments.
Run:
🏁 Script executed:
#!/bin/bash
# Check all import sites for graphology libs and ensure only ESM-style imports are used
rg -nC2 -e "from 'graphology" -e "from 'graphology-(dag|traversal)'" --type=ts
# If unsure about module format, consult upstream quickly:Length of output: 546
🌐 Web query:
Do graphology@0.26.0, graphology-dag@0.4.1, and graphology-traversal@0.3.1 support ESM imports in Node when package.json has "type":"module"?
Error during web search
🏁 Script executed:
#!/bin/bash
set -e
echo "graphology@0.26.0 package.json relevant fields:"
curl -s https://unpkg.com/graphology@0.26.0/package.json | grep -E '"(main|module|exports)"' -B1 -A5
echo
echo "graphology-dag@0.4.1 package.json relevant fields:"
curl -s https://unpkg.com/graphology-dag@0.4.1/package.json | grep -E '"(main|module|exports)"' -B1 -A5
echo
echo "graphology-traversal@0.3.1 package.json relevant fields:"
curl -s https://unpkg.com/graphology-traversal@0.3.1/package.json | grep -E '"(main|module|exports)"' -B1 -A5
echo
echo "Import usage of graphology-traversal:"
rg -n "graphology-traversal" -C2 --type=ts || echo "No imports found"Length of output: 1755
Graphology packages ESM/CJS support and required import adjustments
Based on the published package manifests:
-
graphology@0.26.0
• Provides dual‐format builds:
– “module”: dist/graphology.esm.js
– “exports.import”: dist/graphology.mjs
– “require”: dist/graphology.cjs.js
→ staticimport Graph from 'graphology'will load the ESM build under"type":"module". -
graphology-dag@0.4.1 & graphology-traversal@0.3.1
• Only declare"main": "index.js"with no"module"or"exports"entries
→ these are CJS‐only packages. Static ESM named imports (e.g.import { topologicalSort } from 'graphology-dag') may fail under Node’s ESM loader or in Bun.
Action items:
- Confirm in your target runtimes (Node ≥14+, Bun) that all import sites still resolve as expected under
"type":"module". - For CJS-only packages, switch to a default‐then‐destructure pattern if named imports error:
import dag from 'graphology-dag'; const { topologicalSort } = dag;
- Alternatively, use dynamic
import('graphology-dag')or adjust bundler settings to enable CJS–ESM interop where necessary.
Please update import statements or add compatibility shims so that the three packages load reliably in both Node and Bun under ESM.
🤖 Prompt for AI Agents
In package.json around lines 36 to 41, the review points out graphology is
dual-format ESM/CJS while graphology-dag and graphology-traversal are CJS-only,
which can break static ESM named imports under "type":"module"; update code so
imports are compatible: confirm behavior on target runtimes (Node ≥14+, Bun) and
replace any static named ESM imports from the CJS-only packages with a
default-then-destructure pattern or use dynamic import() where used, or add
bundler/CJS-interop shim; ensure any import sites that rely on named imports are
updated to first import the default and then destructure (or use await
import(...) in async contexts) so the packages load reliably under ESM.
| for (const typeParam of interfaceDecl.getTypeParameters()) { | ||
| const constraint = typeParam.getConstraint() | ||
| if (constraint) { | ||
| traverse(constraint) | ||
| } | ||
| } | ||
|
|
||
| for (const heritageClause of interfaceDecl.getHeritageClauses()) { | ||
| for (const typeNode of heritageClause.getTypeNodes()) { | ||
| traverse(typeNode) | ||
| } | ||
| } | ||
|
|
||
| return references | ||
| } |
There was a problem hiding this comment.
Interface dependency extraction ignores member types
Currently only constraints/heritage are traversed; property/method/index signature types are missed, breaking dependency detection.
for (const typeParam of interfaceDecl.getTypeParameters()) {
const constraint = typeParam.getConstraint()
if (constraint) {
traverse(constraint)
}
}
for (const heritageClause of interfaceDecl.getHeritageClauses()) {
for (const typeNode of heritageClause.getTypeNodes()) {
traverse(typeNode)
}
}
+ // Traverse interface members (properties, methods, index/call signatures, etc.)
+ for (const member of interfaceDecl.getMembers()) {
+ member.forEachChild(traverse)
+ }📝 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.
| for (const typeParam of interfaceDecl.getTypeParameters()) { | |
| const constraint = typeParam.getConstraint() | |
| if (constraint) { | |
| traverse(constraint) | |
| } | |
| } | |
| for (const heritageClause of interfaceDecl.getHeritageClauses()) { | |
| for (const typeNode of heritageClause.getTypeNodes()) { | |
| traverse(typeNode) | |
| } | |
| } | |
| return references | |
| } | |
| for (const typeParam of interfaceDecl.getTypeParameters()) { | |
| const constraint = typeParam.getConstraint() | |
| if (constraint) { | |
| traverse(constraint) | |
| } | |
| } | |
| for (const heritageClause of interfaceDecl.getHeritageClauses()) { | |
| for (const typeNode of heritageClause.getTypeNodes()) { | |
| traverse(typeNode) | |
| } | |
| } | |
| // Traverse interface members (properties, methods, index/call signatures, etc.) | |
| for (const member of interfaceDecl.getMembers()) { | |
| member.forEachChild(traverse) | |
| } | |
| return references |
🤖 Prompt for AI Agents
In src/traverse/dependency-traversal.ts around lines 111 to 125, the interface
traversal only walks type parameter constraints and heritage clauses but skips
member signatures, so property types, method parameter/return types, index
signatures and call signatures are not traversed; update the function to iterate
over interfaceDecl.getMembers() and for each member handle PropertySignature
(traverse its typeNode), MethodSignature (traverse parameter types, return type,
and its type parameters' constraints), IndexSignature (traverse its parameter
types and return type) and CallSignature similarly, and for any member
recursively traverse any nested TypeNode(s) found (including
mapped/tuple/array/union/intersection nodes) so all referenced types are
captured before returning references.
| for (const typeAlias of typeAliases) { | ||
| typeAliasMap.set(typeAlias.getName(), typeAlias) | ||
| this.dependencyGraph.addNode(typeAlias.getName(), { | ||
| type: 'typeAlias', | ||
| declaration: typeAlias, | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing node attributes for type aliases during analysis lead to undefined flags later
Nodes added here lack sourceFile and isImported, causing downstream consumers to see undefined. Populate them.
- this.dependencyGraph.addNode(typeAlias.getName(), {
- type: 'typeAlias',
- declaration: typeAlias,
- })
+ this.dependencyGraph.addNode(typeAlias.getName(), {
+ type: 'typeAlias',
+ declaration: typeAlias,
+ sourceFile: typeAlias.getSourceFile(),
+ isImported: false,
+ })📝 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.
| for (const typeAlias of typeAliases) { | |
| typeAliasMap.set(typeAlias.getName(), typeAlias) | |
| this.dependencyGraph.addNode(typeAlias.getName(), { | |
| type: 'typeAlias', | |
| declaration: typeAlias, | |
| }) | |
| } | |
| for (const typeAlias of typeAliases) { | |
| typeAliasMap.set(typeAlias.getName(), typeAlias) | |
| this.dependencyGraph.addNode(typeAlias.getName(), { | |
| type: 'typeAlias', | |
| declaration: typeAlias, | |
| sourceFile: typeAlias.getSourceFile(), | |
| isImported: false, | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src/traverse/dependency-traversal.ts around lines 259-266, the nodes added
for type aliases lack the sourceFile and isImported attributes; update the
object passed to this.dependencyGraph.addNode(...) to include sourceFile: the
typeAlias.getSourceFile() (or its file path) and isImported: a boolean
indicating whether this alias comes from an external/imported file (compute by
comparing the alias's source file path to the current file being processed or by
using any existing helper like isImportedNode). Ensure both values are set for
every typeAlias so downstream consumers don't receive undefined.
| const dependencyAnalysis = dependencyTraversal.analyzeProcessingOrder( | ||
| localTypeAliases, | ||
| interfaces, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Analysis mutates traversal graph; reset before collection to avoid stale/incomplete node attributes
analyzeProcessingOrder populates the graph with type aliases lacking sourceFile/isImported. Subsequent collection won’t update existing nodes, causing getTopologicallySortedTypes to yield undefined flags.
const dependencyAnalysis = dependencyTraversal.analyzeProcessingOrder(
localTypeAliases,
interfaces,
)
+
+ // Reset graph state before collection to ensure nodes carry complete attributes.
+ dependencyTraversal.clearCache()📝 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 dependencyAnalysis = dependencyTraversal.analyzeProcessingOrder( | |
| localTypeAliases, | |
| interfaces, | |
| ) | |
| const dependencyAnalysis = dependencyTraversal.analyzeProcessingOrder( | |
| localTypeAliases, | |
| interfaces, | |
| ) | |
| // Reset graph state before collection to ensure nodes carry complete attributes. | |
| dependencyTraversal.clearCache() |
🤖 Prompt for AI Agents
In src/ts-morph-codegen.ts around lines 78-81, the call to
dependencyTraversal.analyzeProcessingOrder mutates the traversal graph and
leaves nodes with missing sourceFile/isImported, causing later collection to see
stale/undefined flags; fix by resetting or recreating the traversal graph before
performing the collection step (e.g., call a provided reset/clear method on
dependencyTraversal or instantiate a new traversal instance after
analyzeProcessingOrder) so that getTopologicallySortedTypes operates on a fresh
graph with correct node attributes.
This commit introduces a comprehensive type dependency traversal implementation
that combines AST traversal, dependency collection, and analysis. The key
changes are:
DependencyTraversalclass that manages the dependency graph usingGraphology library
declarations
TypeReferenceExtractorinterface to allow customization of typereference extraction
ProcessingOrderResultto analyze the optimal processing order forinterfaces and type aliases
These changes enable efficient management of type dependencies, which is
crucial for generating code that respects the correct order of type
declarations.