fix(nitrogen): handle cyclic struct references#1074
Open
puckey wants to merge 9 commits into
Open
Conversation
Previously, struct types that reference themselves (e.g., `Node { node?: Node }`)
or have indirect cycles (e.g., `Foo { bar: Bar }`, `Bar { foo: Foo }`) would cause
"Maximum call stack size exceeded" during code generation.
This fix introduces:
- Cycle detection in createType.ts using a processingTypes set
- Lazy initialization in StructType to break cyclic dependencies
- visited Set parameter in getRequiredImports/getExtraFiles to prevent infinite recursion
Includes test cases for both direct self-reference and indirect cycles.
Adds a struct with a callback that references the same struct type. Currently fails with "Maximum call stack size exceeded" due to infinite recursion in nitrogen's type walker.
Add cycle detection for struct types that directly or indirectly reference themselves. Allow self-references through reference types (arrays, maps, callbacks, promises) since these use heap allocation. Removes Cyclic.nitro.ts test file and adds proper test cases (TreeNode, TreeNodeMap, SelfReferentialStruct) to TestObject.nitro.ts.
Explain that direct/indirect cyclic struct references are not supported, but self-references through arrays, maps, or functions are allowed since these are reference types using heap allocation.
Generated files for TreeNode, TreeNodeMap, and SelfReferentialStruct test cases demonstrating allowed self-reference patterns.
When a struct contains a function that references the same struct (e.g., `transform?: (config: MyStruct) => Promise<MyStruct>`), the generated JNI headers would have circular includes. Generate valid C++ by using forward declarations, deferring includes until after class declarations, and defining methods out-of-line. Extract shared cyclic detection logic into detectCyclicDependencies.ts.
Add runtime tests for bounceSelfReferentialStruct, bounceTreeNode, and bounceTreeNodeMap to verify the cyclic struct handling works correctly on device.
Properly separate declarations from implementations for cyclic struct/function dependencies by generating separate .hpp and .cpp files instead of inline deferred definitions. Also consolidate import partitioning logic into a single `partitionAndTransformImports` helper function.
|
Someone is attempting to deploy a commit to the Margelo Team on Vercel. A member of the Team first needs to authorize it. |
puckey
changed the title
This was referenced Dec 2, 2025
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
|
Hey - I think overall the change is good - it's making DX better. I really appreciate you taking the time to work on this. ❤️ I am just not a big fan of how complex this is to implement - not only does it touch a lot of the codebase, but also it passes the We can leave this PR open for future reference, but I was hoping for a much smaller change... |
puckey
added a commit
to puckey/nitro
that referenced
this pull request
Mar 2, 2026
Structs that reference themselves through reference types (arrays, callbacks, promises, records) previously caused a stack overflow during code generation. Break the cycle by tracking structs being initialized and using lazy property resolution in StructType. When a struct is encountered while it's already being processed, the cached instance is returned instead of recursing infinitely. Follows up on PR mrousavy#1074 with a simpler approach per mrousavy's feedback about reducing complexity. Fixes mrousavy#1070
Contributor
Author
|
Thanks for the feedback! I've reworked this into two smaller, focused PRs:
Happy to close this one in favour of those. |
puckey
added a commit
to puckey/nitro
that referenced
this pull request
Mar 2, 2026
Structs that reference themselves through reference types (arrays, callbacks, promises, records) previously caused a stack overflow during code generation. Break the cycle by tracking structs being initialized and using lazy property resolution in StructType. When a struct is encountered while it's already being processed, the cached instance is returned instead of recursing infinitely. Follows up on PR mrousavy#1074 with a simpler approach per mrousavy's feedback about reducing complexity. Fixes mrousavy#1070
puckey
added a commit
to puckey/nitro
that referenced
this pull request
Mar 2, 2026
Structs that reference themselves through reference types (arrays, callbacks, promises, records) previously caused a stack overflow during code generation. Break the cycle by tracking structs being initialized and using lazy property resolution in StructType. When a struct is encountered while it's already being processed, the cached instance is returned instead of recursing infinitely. Follows up on PR mrousavy#1074 with a simpler approach per mrousavy's feedback about reducing complexity. Fixes mrousavy#1070
puckey
added a commit
to puckey/nitro
that referenced
this pull request
Mar 4, 2026
Structs that reference themselves through reference types (arrays, callbacks, promises, records) previously caused a stack overflow during code generation. Break the cycle by tracking structs being initialized and using lazy property resolution in StructType. When a struct is encountered while it's already being processed, the cached instance is returned instead of recursing infinitely. Follows up on PR mrousavy#1074 with a simpler approach per mrousavy's feedback about reducing complexity. Fixes mrousavy#1070
puckey
added a commit
to puckey/nitro
that referenced
this pull request
Jun 12, 2026
Structs that reference themselves through reference types (arrays, callbacks, promises, records) previously caused a stack overflow during code generation. Break the cycle by tracking structs being initialized and using lazy property resolution in StructType. When a struct is encountered while it's already being processed, the cached instance is returned instead of recursing infinitely. Follows up on PR mrousavy#1074 with a simpler approach per mrousavy's feedback about reducing complexity. Fixes mrousavy#1070
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.hpp/.cppfiles for types with cyclic dependencies to avoid circular includes (Android)visitedset parameter to prevent infinite recursion in type walkers (affects all platforms)Test plan
SelfReferentialStructtest case with callback that returns the same struct typeTreeNodeandTreeNodeMaptest cases for tree-like data structuresNote
This PR was implemented with Claude Code. I'm not deeply familiar with all the C++/JNI internals here, but did my best to follow along and verify the changes make sense. Also tested it in library code I am working on.