Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type computation for many optional properties #1473

Merged
merged 2 commits into from May 14, 2024

Conversation

msujew
Copy link
Contributor

@msujew msujew commented Apr 25, 2024

Closes #775

Essentially uses a method to compute subgraphs of a type (i.e. the hull between a TypePart and its own end property) and merging them early to prevent exponential runtime behavior. The merging happens every time a subgraph has been successfully traversed (at the end of the recurse method).

Since multiple subgraphs can be chained together, we need a new iterate method that ensures that all subgraphs are traversed to the end of the type.

@msujew msujew added the types Types related issue label Apr 25, 2024
Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one thing that stands out is the way ends is being used in recurse, that could probably stand to be changed. However beyond that the rest looks okay. I can see the issue before that's resolved by avoiding the repeated applyNext loop, and looks to take care of it. But I can't say if there's something else with regards to whether ends was intended to be an array for a specific reason.

@msujew msujew requested a review from montymxb May 8, 2024 11:29
Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:octocat: Approved, looks good

@msujew msujew merged commit a00974d into main May 14, 2024
5 checks passed
@msujew msujew deleted the msujew/fix-deeply-nested-types branch May 14, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types Types related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maximum call stack size exceeded when using many optional groups
2 participants