feat: Upgrade dependencies and refactor chunk creation #25
Conversation
- Upgrade `@eslint/js`, `@typescript-eslint/eslint-plugin`, `@typescript-eslint/parser`, and `eslint` to their latest versions - Refactor the `createChunkNodes` function in `local-type-collector.ts` to use the new `createChunkNodes` and `shouldChunkUnion` functions from the `chunk-large-types` module - This change improves the handling of large union types by breaking them into smaller chunks, making the generated code more manageable and easier to work with
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new chunking module to split very large union types into smaller synthetic chunk types, integrates chunk creation into import and local-type collection, ensures chunks are marked required during dependency extraction, updates ImportCollector initialization to accept node ID sets, updates devDependencies, and adds TypeBox "as const" tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant IC as ImportCollector
participant TS as ts-morph
participant CLT as chunk-large-types
participant NG as NodeGraph
participant RS as resolverStore
participant Sets as maincode/required Sets
IC->>TS: Inspect type alias type node
IC->>CLT: shouldChunkUnion(typeNode)?
alt large union
IC->>CLT: createChunkNodes(node, parentName, NG, Sets.maincode, Sets.required, sourceFile, addToRequired=false)
CLT->>TS: Build synthetic chunk SourceFile & TypeNodes
CLT->>RS: Register chunk qualified names & type mappings
CLT->>NG: Add chunk TraversedNodes
CLT-->>IC: return chunkReferences[]
IC->>NG: Add dependencies (chunk -> alias) and store alias with chunkReferences
else small/normal
IC->>NG: Add alias node (no chunking)
end
sequenceDiagram
autonumber
participant DE as DependencyExtractor
participant RS as resolverStore
participant NG as NodeGraph
participant CLT as chunk-large-types
participant Sets as requiredNodeIds
loop traverse dependencies
DE->>RS: hasQualifiedName(referencedType)?
DE->>NG: hasNode(referencedType)?
alt newly discovered type
DE->>Sets: add(referencedType)
DE->>CLT: markChunksAsRequired(referencedType, NG, Sets)
CLT->>NG: read parent node.chunkReferences
CLT->>Sets: add(each chunkReference)
DE->>DE: queue nodesToProcess (including chunks)
else already known
DE-->>DE: continue
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
41-58: Action: fix ESLint/TS-ESLint compatibility and devDeps
- Node: ESLint 9.x and typescript-eslint 8.x require Node >= 18.18 — ensure CI/runtime uses Node >=18.18. (npmjs.com, tseslint.com)
- TypeScript: package.json has no typescript devDependency (jq showed "typescript not listed"); add a TypeScript devDependency that is compatible with @typescript-eslint (typescript-eslint supported TS range per their docs). Also note the registry latest TypeScript is 5.9.2 — pin a supported TS version or ensure the workspace provides it. (tseslint.com, npmjs.com)
- jiti: ESLint requires jiti to load eslint.config.ts (error seen when jiti missing). jiti is in devDependencies — ensure CI installs devDependencies before running ESLint and that jiti is available. (eslint.org)
- Keep globals: eslint.config.ts imports globals (eslint.config.ts:4 and 13).
- graphology-types: no references found in .ts/.js — remove if unused.
Fixes: add a compatible "typescript" devDependency to package.json, ensure CI uses Node >=18.18 and installs devDependencies before linting, and drop any truly-unused devDeps (graphology-types).
🧹 Nitpick comments (8)
tests/handlers/typebox/as-const.test.ts (2)
12-71: Reduce duplication via table-driven tests for tuple cases.You can collapse the five array cases into a parametric loop to keep the suite terse.
Example:
- describe('array literals with as const', () => { - test('string literals array', () => { /* ... */ }) - test('numeric literals array', () => { /* ... */ }) - test('boolean literals array', () => { /* ... */ }) - test('mixed literals array', () => { /* ... */ }) - test('empty array', () => { /* ... */ }) - }) + describe('array literals with as const', () => { + const cases: Array<[name: string, expr: string, tuple: string]> = [ + ['string literals array', '["hello", "world"]', 'Type.Tuple([Type.Literal("hello"), Type.Literal("world")])'], + ['numeric literals array', '[1, 2, 3]', 'Type.Tuple([Type.Literal(1), Type.Literal(2), Type.Literal(3)])'], + ['boolean literals array', '[true, false]', 'Type.Tuple([Type.Literal(true), Type.Literal(false)])'], + ['mixed literals array', '["hello", 42, true]', 'Type.Tuple([Type.Literal("hello"), Type.Literal(42), Type.Literal(true)])'], + ['empty array', '[]', 'Type.Tuple([])'], + ] + for (const [name, value, tuple] of cases) { + test(name, () => { + const sf = createSourceFile(project, `type A = ${value} as const`) + expect(generateFormattedCode(sf)).toBe( + formatWithPrettier(` + export const A = ${tuple}; + export type A = Static<typeof A>; + `), + ) + }) + } + })
111-124: Add a regression for “indexed access of const arrays” to guard chunking path.Covers typeof arr[number] as const, which routes through the IndexedAccessType codepath used by the chunker.
Example test to append:
test('indexed access over const array', () => { const sf = createSourceFile(project, ` const arr = ["a","b","c"] as const type A = (typeof arr)[number] as const `) expect(generateFormattedCode(sf)).toBe( formatWithPrettier(` export const A = Type.Union([Type.Literal("a"), Type.Literal("b"), Type.Literal("c")]); export type A = Static<typeof A>; `), ) })src/traverse/chunk-large-types.ts (1)
6-6: Make CHUNK_SIZE configurable/exported.Exporting CHUNK_SIZE helps targeted tests and tuning without touching internals.
-const CHUNK_SIZE = 20 +export const CHUNK_SIZE = 20src/traverse/dependency-extractor.ts (2)
54-56: Also enqueue chunk nodes for dependency extractionMarking chunk refs as required is good, but without enqueuing them, any nested refs inside chunk nodes won't be discovered. Add them to
nodesToProcesstoo.requiredNodeIds.add(referencedType) nodesToProcess.add(referencedType) // Mark chunks as required if this type has chunks markChunksAsRequired(referencedType, nodeGraph, requiredNodeIds) + // Also enqueue chunk nodes so their internal references (if any) are extracted + const parentNode = nodeGraph.getNode(referencedType) + if (parentNode?.chunkReferences) { + for (const chunkRef of parentNode.chunkReferences) { + if (!requiredNodeIds.has(chunkRef)) { + nodesToProcess.add(chunkRef) + } + } + }
50-50: Fix misleading comment (processed → required)This checks membership in
requiredNodeIds, not whether the node was processed.- // Only add to required if not already processed + // Only add to required if not already requiredsrc/traverse/import-collector.ts (1)
55-96: Chunking path looks solid; confirm intent for imported chunks to be treated as main code
- Behavior makes sense: create chunks, attach refs to parent, add edges chunk→parent, and defer marking as required.
- Confirm that chunk nodes created for imported types are intended to be emitted as main code (
isMainCode: trueincreateChunkNodes). If that’s the design, all good; otherwise we may need a flag to control this.Optional: consider guarding against project memory growth from many temp chunk files created in
createChunkNodes(synthetic files accumulate). A reusable scratch file or cleanup strategy would help.Also applies to: 81-85
src/traverse/local-type-collector.ts (2)
20-41: Empty-locals fallback only handles named imports; confirm default/namespace cases.If a file exports types via a default or namespace import, they’ll be skipped here. If that’s intentional (handled elsewhere), all good; otherwise, consider extending coverage.
What scenarios do we expect here?
- default export type alias (e.g.,
export default type Foo = ...)- namespace import (
import * as X from ...) with types insideIf any should be treated as “required,” I can draft a safe extension that won’t break
resolverStore.generateQualifiedName. Do you want that?
84-89: Use the same sourceFile accessor for consistency.Elsewhere you pass
decl.getSourceFile(). Here you pass the outersourceFile. Same value, but consistency helps static analysis and future refactors.Apply:
- resolverStore.addTypeMapping({ - originalName: typeName, - sourceFile, - }) + resolverStore.addTypeMapping({ + originalName: typeName, + sourceFile: typeAlias.getSourceFile(), + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
package.json(2 hunks)src/traverse/chunk-large-types.ts(1 hunks)src/traverse/dependency-extractor.ts(2 hunks)src/traverse/dependency-traversal.ts(1 hunks)src/traverse/import-collector.ts(3 hunks)src/traverse/local-type-collector.ts(1 hunks)tests/handlers/typebox/as-const.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/traverse/chunk-large-types.ts (3)
src/traverse/node-graph.ts (1)
NodeGraph(8-38)src/utils/resolver-store.ts (1)
resolverStore(174-174)src/traverse/types.ts (1)
TraversedNode(3-14)
tests/handlers/typebox/as-const.test.ts (1)
tests/utils.ts (3)
createSourceFile(12-14)generateFormattedCode(26-37)formatWithPrettier(16-24)
src/traverse/dependency-extractor.ts (1)
src/traverse/chunk-large-types.ts (1)
markChunksAsRequired(106-117)
src/traverse/dependency-traversal.ts (1)
src/traverse/import-collector.ts (1)
ImportCollector(10-183)
src/traverse/import-collector.ts (1)
src/traverse/chunk-large-types.ts (2)
shouldChunkUnion(8-23)createChunkNodes(25-104)
🔇 Additional comments (11)
tests/handlers/typebox/as-const.test.ts (1)
5-10: Test scaffolding is solid.Project lifecycle with beforeEach is clean and isolation-friendly.
src/traverse/chunk-large-types.ts (2)
36-45: Confirm Type.getText() stability for IndexedAccess unions.Type.getText() may emit import("...") references or contextual names; ensure the printer produces valid code in your generator.
Add a regression that chunks a large
typeof arr[number]and validates emitted chunk text doesn’t include opaque import() types. If it does, consider resolving symbols to fully qualified names or using a printer with the parent source file’s context.
97-101: Double-check resolver mapping for synthetic chunks.You map originalName to the parent’s sourceFile, while the node lives in a scratch file. Validate this doesn’t collide with real types and that imports won’t be generated for chunks (since isImported=false, isMainCode=true).
If collisions occur, pass the qualifiedName/alias explicitly to the resolver, or add a dedicated “synthetic” namespace to resolverStore.
src/traverse/dependency-extractor.ts (1)
1-1: LGTM: importing markChunksAsRequired integrates chunking into dependency markingNo issues.
src/traverse/dependency-traversal.ts (1)
20-25: LGTM: pass chunking context into ImportCollectorConstructor injection of the sets keeps state centralized and consistent across traversal phases.
src/traverse/import-collector.ts (2)
1-4: LGTM: imports for chunking utilitiesMatches usage below.
14-16: LGTM: extended constructor signaturePassing
maincodeNodeIdsandrequiredNodeIdsaligns with downstream chunk creation.src/traverse/local-type-collector.ts (4)
91-114: Interfaces collection looks solid.IDs are added to both sets and resolver mapping is recorded. No issues spotted.
116-136: Enums collection LGTM.Consistent with interfaces block; no functional concerns.
138-164: Functions collection LGTM, with a sensible guard for unnamed declarations.No issues spotted.
50-73: NodeGraph already supportschunkReferences
TheTraversedNodeinterface insrc/traverse/types.tsdeclareschunkReferences?: string[], andaddTypeNodestores the node object intact. Downstream parsers and printers already handle this field. No action required.
| export const markChunksAsRequired = ( | ||
| parentQualifiedName: string, | ||
| nodeGraph: NodeGraph, | ||
| requiredNodeIds: Set<string>, | ||
| ): void => { | ||
| const parentNode = nodeGraph.getNode(parentQualifiedName) | ||
| if (parentNode?.chunkReferences) { | ||
| for (const chunkRef of parentNode.chunkReferences) { | ||
| requiredNodeIds.add(chunkRef) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
getNode() will throw when parent hasn’t been added — add a guard.
nodeGraph.getNode throws for unknown nodes; current optional chaining won’t help.
Apply:
export const markChunksAsRequired = (
parentQualifiedName: string,
nodeGraph: NodeGraph,
requiredNodeIds: Set<string>,
): void => {
- const parentNode = nodeGraph.getNode(parentQualifiedName)
- if (parentNode?.chunkReferences) {
- for (const chunkRef of parentNode.chunkReferences) {
- requiredNodeIds.add(chunkRef)
- }
- }
+ if (!nodeGraph.hasNode(parentQualifiedName)) return
+ const parentNode = nodeGraph.getNode(parentQualifiedName)
+ const refs = parentNode.chunkReferences
+ if (!refs?.length) return
+ for (const chunkRef of refs) {
+ requiredNodeIds.add(chunkRef)
+ }
}📝 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 markChunksAsRequired = ( | |
| parentQualifiedName: string, | |
| nodeGraph: NodeGraph, | |
| requiredNodeIds: Set<string>, | |
| ): void => { | |
| const parentNode = nodeGraph.getNode(parentQualifiedName) | |
| if (parentNode?.chunkReferences) { | |
| for (const chunkRef of parentNode.chunkReferences) { | |
| requiredNodeIds.add(chunkRef) | |
| } | |
| } | |
| } | |
| export const markChunksAsRequired = ( | |
| parentQualifiedName: string, | |
| nodeGraph: NodeGraph, | |
| requiredNodeIds: Set<string>, | |
| ): void => { | |
| if (!nodeGraph.hasNode(parentQualifiedName)) return | |
| const parentNode = nodeGraph.getNode(parentQualifiedName) | |
| const refs = parentNode.chunkReferences | |
| if (!refs?.length) return | |
| for (const chunkRef of refs) { | |
| requiredNodeIds.add(chunkRef) | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/traverse/chunk-large-types.ts around lines 106 to 117, nodeGraph.getNode
can throw for unknown nodes so the current optional chaining won’t prevent the
exception; add a guard before calling getNode (e.g. check
nodeGraph.hasNode(parentQualifiedName) or wrap getNode in a try/catch) and only
access chunkReferences when a node is confirmed to exist, then add its
chunkReferences to requiredNodeIds as before.
| import { | ||
| createChunkNodes, | ||
| shouldChunkUnion, | ||
| } from '@daxserver/validation-schema-codegen/traverse/chunk-large-types' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm deep import path + named exports resolve correctly.
Verify that chunk-large-types actually exports createChunkNodes and shouldChunkUnion, and that the @daxserver/validation-schema-codegen/* path alias is configured (tsconfig paths or package exports) to avoid resolution/circular-import issues.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) File exists:"
fd -a 'chunk-large-types.ts' src
echo -e "\n2) Named exports present:"
rg -nP 'export\s+(?:const|function)\s+(createChunkNodes|shouldChunkUnion)\b' src
echo -e "\n3) Path alias configured:"
rg -nP '"paths"\s*:\s*{[^}]*"@daxserver/validation-schema-codegen/\*"' -n tsconfig*.json -C2 || true
echo -e "\n4) Check for potential circular imports:"
rg -nP 'from\s+[\'"]@daxserver/validation-schema-codegen/traverse/local-type-collector[\'"]' -n src || trueLength of output: 865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tsconfig files:"
fd -HI tsconfig*.json
echo -e "\nPaths config in tsconfig files:"
for f in $(fd -HI tsconfig*.json); do
echo "File: $f"
rg -nP '"paths"\s*:' -n "$f" -C2 || echo "No paths"
done
echo -e "\nExport fields in package.json:"
rg -nP '"exports"\s*:' -n package.json -C3 || echo "No exports field"
echo -e "\nImports in src/traverse/chunk-large-types.ts:"
rg -n '^import' -n src/traverse/chunk-large-types.ts
echo -e "\nCheck for circular import from chunk-large-types to local-type-collector via alias:"
rg -nP "from\s+['\"]@daxserver/validation-schema-codegen/traverse/local-type-collector['\"]" -n src || echo "None"Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Listing tsconfig files:"
ls tsconfig.json tsconfig.build.json 2>/dev/null || echo "No tsconfig*.json files found"
for file in tsconfig.json tsconfig.build.json; do
if [ -f "$file" ]; then
echo -e "\n== $file =="
echo "1) 'paths' section:"
rg -nP '"paths"\s*:' -n "$file" -C5 || echo " – no 'paths' section"
echo "2) '@daxserver/validation-schema-codegen/*' alias:"
rg -nP '"@daxserver/validation-schema-codegen/\*"' -n "$file" -C5 || echo " – no alias for validation-schema-codegen"
else
echo "$file not present"
fi
done
echo -e "\nChecking package.json exports:"
if [ -f package.json ]; then
rg -nP '"exports"\s*:' -n package.json -C5 || echo " – no 'exports' field in package.json"
rg -nP '"\./traverse/chunk-large-types"' -n package.json -C5 || echo " – no direct export for chunk-large-types"
else
echo "package.json not found"
fi
echo -e "\nSearching for circular import back to local-type-collector:"
rg -nP "from\s+['\"]@daxserver/validation-schema-codegen/traverse/local-type-collector['\"]" -n src || echo " – no circular imports detected"Length of output: 2842
Expose deep imports and include path aliases in build config
- package.json: add exports for
"./traverse/chunk-large-types"(and other deep paths) pointing to the corresponding./dist/traverse/*.jsand.d.tsfiles - tsconfig.build.json: extend tsconfig.json or replicate its
"paths": { "@daxserver/validation-schema-codegen/*": ["./src/*"] }so deep imports resolve during build
🤖 Prompt for AI Agents
In src/traverse/local-type-collector.ts around lines 1 to 4, the reviewer asks
that deep imports like
"@daxserver/validation-schema-codegen/traverse/chunk-large-types" be exposed and
resolvable during build; update package.json to add exports entries for
"./traverse/chunk-large-types" (and any other deep traverse paths) mapping to
their built files under ./dist/traverse/*.js and corresponding .d.ts, and modify
tsconfig.build.json to extend or include the same "paths": {
"@daxserver/validation-schema-codegen/*": ["./src/*"] } (or replicate it) so the
deep import path aliases resolve when compiling the project.
@eslint/js,@typescript-eslint/eslint-plugin,@typescript-eslint/parser, andeslintto their latest versionscreateChunkNodesfunction inlocal-type-collector.tsto use the newcreateChunkNodesandshouldChunkUnionfunctions from thechunk-large-typesmodule