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 MergeDeep in TypeScript 5.4 #806

Closed
wants to merge 2 commits into from
Closed

Conversation

voxpelli
Copy link
Collaborator

After the change in microsoft/TypeScript#56004 the PickRestType was causing some types to be considered theoretically infinite in their recursion.

I can't wrap my mind around exactly why right now, but I can see how PickRestType on a crazy long tuple will cause a very deep recursion, so I decided to simply cap its recursion and return unknown[] if it goes past that point and that solved the errors.

Side effects:

  • In practice: rarely any I believe
  • In theory: some merged types will regress to unknown[] in TypeScript versions older than 5.4

Considerations:

Is the DepthTracker extends Array<true> = [] + PickRestTypeMaxDepth extends DepthTracker['length'] a good way to track depth? I know I have done a depth check previous times but can't remember what solution I picked then.

Fixes #784

After the change in microsoft/TypeScript#56004 the `PickRestType` was causing some types to be considered theoretically infinite in their recursion.

I can't wrap my mind around exactly why right now, but I can see how `PickRestType` on a crazy long tuple will cause a very deep recursion, so I decided to simply cap its recursion and return `unknown[]` if it goes past that point and that solved the errors.

Side effects:

- In practice: rarely any I believe
- In theory: some merged types will regress to `unknown[]` in TypeScript versions older than 5.4

Considerations:

Is the `DepthTracker extends Array<true> = []` + `PickRestTypeMaxDepth extends DepthTracker['length']` a good way to track depth? I know I have done a depth check previous times but can't remember what solution I picked then.

Fixes #784
@voxpelli
Copy link
Collaborator Author

Here's a canary run on this branch that shows that it fixes the problem: https://github.com/sindresorhus/type-fest/actions/runs/7697319193

@skarab42
Copy link
Collaborator

sorry not enough time to go further but #807 seems to fix the bug and improve performance

@skarab42
Copy link
Collaborator

@voxpelli voxpelli deleted the typescript54-compatibility branch January 30, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canary: MergeDeep failures in typescript@next not detected by canary tests
3 participants