fix: omit utility type with recursive interfaces causing stack overflow#2426
fix: omit utility type with recursive interfaces causing stack overflow#2426Copilot wants to merge 11 commits into
Conversation
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
… in heritage clauses Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
…essages, add clarifying comments Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
…irst Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
|
@copilot CI is failing, fix issues |
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
| if (typeSymbol && typeSymbol.flags & ts.SymbolFlags.Alias) { | ||
| const aliasedSymbol = this.typeChecker.getAliasedSymbol(typeSymbol); | ||
| // Check if any declaration is from a TypeScript lib file | ||
| // Lib utility types (Omit, Pick, etc.) should be skipped to prevent |
There was a problem hiding this comment.
Let's test that omit and pick if not recursive work well even after this change.
There was a problem hiding this comment.
Existing tests for non-recursive Omit and Pick already exist and pass:
test/valid-data/type-conditional-omit- testsOmit<Test, "b" | "d">test/valid-data/type-mapped-pick-union-alias- testsPick<SomeInterface, ABCD | E>
These verify that the fix doesn't break non-recursive usage of these utility types.
| (result: BaseType[], baseType) => [ | ||
| ...result, | ||
| ...baseType.types.map((expression) => this.childNodeParser.createType(expression, context)), | ||
| ...baseType.types |
There was a problem hiding this comment.
This file has rather involved changes. Are they really required since the error seems to be a regression (and the previous code worked).
There was a problem hiding this comment.
The core change is minimal - it adds a filter to skip TypeScript lib utility types in heritage clauses to prevent infinite recursion. The involved formatting is due to:
- Converting simple
.map()to.map().filter()chain - Adding the lib type detection logic inside the map
The alternative would be improving circular reference detection to handle the recursive type + Omit case, which would be a much larger change. This approach prevents the parser from following into the internal mapped type implementations of lib utility types (Omit, Pick, etc.) which was causing the recursion.
Co-authored-by: arthurfiorette <47537704+arthurfiorette@users.noreply.github.com>
Fixed all review comments in commit c646d44:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression introduced in v2.4.0 where using Omit utility type with recursive interfaces causes a "maximum call stack size exceeded" error. The fix prevents infinite recursion by skipping TypeScript standard library utility types (like Omit, Pick, etc.) when processing interface heritage clauses, allowing the TypeScript compiler's built-in type resolution to handle these cases instead.
Key Changes
- Introduced
isTypeScriptLibFileutility to detect TypeScript standard library files consistently across the codebase - Modified
InterfaceAndClassNodeParserto skip lib utility types in heritage clauses, preventing infinite recursion - Enhanced null safety in
ExpressionWithTypeArgumentsNodeParserusingLogicErrorclass
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
test/valid-data/type-omit-recursive/main.ts |
Comprehensive test cases covering simple and complex recursive interfaces with Omit and Pick utility types |
test/valid-data/type-omit-recursive/schema.json |
Expected JSON schema output showing correct property omission without recursive properties |
test/valid-data/type-omit-recursive/index.test.ts |
Test runner configuration for the new test case |
src/Utils/isTypeScriptLibFile.ts |
New reusable utility function and pattern constant for detecting TypeScript lib files |
src/NodeParser/InterfaceAndClassNodeParser.ts |
Core fix to skip lib utility types in heritage clauses, preventing infinite recursion with recursive types |
src/NodeParser/ExpressionWithTypeArgumentsNodeParser.ts |
Enhanced null safety checks with descriptive error messages using LogicError |
src/NodeParser/AnnotatedNodeParser.ts |
Refactored to use the new isTypeScriptLibFile helper for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@domoritz npm tokens were revoked today, could you migrate ts-json-schema-generator in npm dashboard to use trusted publishing? |
|
This week I'm out of town and would never be able to code such PR in time, I'm loving this copilot feature 🙏 edit: lol it managed to pass tests hahahaha |
|
@copilot please use a semantic pull request title like fix: ... |
The PR commits will be squashed on merge. The suggested semantic commit message for the squashed commit would be: |
|
@domoritz can I merge it? |
|
I don't fully understand the solution. Do you? Why is this change needed when it worked fine in the previous version of this library? |
Summary
Fixed regression where
Omitutility type with recursive interfaces causes "maximum call stack size exceeded" error.Completed Work
Changes
Test Results
✅ All recursive and omit-related tests pass
✅ No regressions in existing tests (type-conditional-omit and type-mapped-pick-union-alias verify non-recursive Omit/Pick work correctly)
✅ No security vulnerabilities detected
✅ Linting passes with no errors
✅ Requires Node 22+ (uses native fs.globSync)
Known Limitation
Complex conditional types with
infer(likeArrayElement<T>) may still cause issues - this is a separate problem with conditional type resolution in circular reference detection.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.