fix: Add tail recursion where it's possible but missing #51
Conversation
This file contains 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
This doesn't seem to affect `selection` benchmarks, but is technically more correct since tail-recursion is possible here. Co-authored-by: deathemperor <[email protected]> Co-authored-by: HaiNNT <[email protected]>
More correct, as per tail recursion in other places, but `variables.ts` didn't get much attention before Co-authored-by: deathemperor <[email protected]> Co-authored-by: HaiNNT <[email protected]>
More correct, as per tail recursion in other places, but `variables.ts` didn't get much attention before
🦋 Changeset detectedLatest commit: 235df22 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
JoviDeCroock
approved these changes
Feb 3, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See #46
Summary
This is partially extracted from #46 and one other opportunity where tail recursion was missing. The changes have been carried over one-by-one and split up.
One change is missing (
UnionAcc) since it doesn't trigger a tail recursion case in TypeScript, since it wasn't on a single type, i.e. non-mutually recursive type alias, but on several types, and hoisted too far. So, that change hasn't been carried over. The benchmarks also seem to confirm that that change isn't relevant for this PR.(Note: In general, the benchmarks, when
selectionis run in isolation, give a good indicator of whether tail recursion is helping)In some of these cases, tail recursion hasn't increased performance, but the opportunity to add them is good nonetheless.
Set of changes
_getPossibleTypeSelectionRectail recursivegetInputObjectTypeRectail recursive_getVariablesRectail recursive