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.

Sign up for GitHub

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

Jump to bottom

Canary: MergeDeep failures in typescript@next not detected by canary tests #784

Closed
voxpelli opened this issue Dec 15, 2023 · 4 comments · Fixed by #807 · May be fixed by diia-open-source/be-types#1 or diia-open-source/be-pkg-test#1
Closed

Canary: MergeDeep failures in typescript@next not detected by canary tests #784

voxpelli opened this issue Dec 15, 2023 · 4 comments · Fixed by #807 · May be fixed by diia-open-source/be-types#1 or diia-open-source/be-pkg-test#1

Comments

@voxpelli
Copy link
Collaborator

voxpelli commented Dec 15, 2023 •
edited by polar-sh bot
Loading

I'm seeing a lot of these style of errors in the canary tests in my own modules:

Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(144,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownArrayOrTuple'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(144,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownRecord'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(144,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Source>>>[number]' and 'UnknownArrayOrTuple'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(170,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownArrayOrTuple'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(170,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownRecord'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(170,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Source>>>[number]' and 'UnknownArrayOrTuple'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(196,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownArrayOrTuple'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(196,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Destination>>>[number]' and 'UnknownRecord'.
Error: ../node_modules/read-pkg/node_modules/type-fest/source/merge-deep.d.ts(196,5): error TS2321: Excessive stack depth comparing types 'PickRestType<ArrayTail<ArrayTail<Source>>>[number]' and 'UnknownArrayOrTuple'.

I didn't receive them in typescript@5.4.0-dev.20231119 then started receiving them in typescript@5.4.0-dev.20231121.

To avoid a regression once typescript@5.4.0 gets launched, we should track down the cause of this and file an issue with typescript if needed.

We should also track down why the canary tests in type-fest itself isn't catching it.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • The funding will be given to active contributors.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@voxpelli voxpelli mentioned this issue Dec 15, 2023
Merged
@voxpelli
Copy link
Collaborator Author

voxpelli commented Dec 15, 2023 •
edited
Loading

The regression should be caused by any of the following commits: microsoft/TypeScript@b4aefd2...13c9b16

Rather, as typescript@5.4.0-dev.20231120 seems to pass as well: microsoft/TypeScript@ffc21e5...13c9b16

@voxpelli voxpelli mentioned this issue Dec 16, 2023
Merged
@voxpelli
Copy link
Collaborator Author

Suggested to add type-fest into typescript's own canary tests: microsoft/typescript-error-deltas#129

@jakebailey
Copy link

FWIW you can pretty easily bisect this using every-ts: https://www.npmjs.com/package/every-ts

$ every-ts fetch
$ every-ts bisect bad 5.4.0-dev.20231121
$ every-ts bisect good 5.4.0-dev.20231119
$ every-ts tsc
$ every-ts bisect bad
$ every-ts tsc
$ every-ts bisect good
...

@voxpelli
Copy link
Collaborator Author

Finally got time to look into this and every-ts is really really great @jakebailey!

Came to the conclusion that its microsoft/TypeScript#56004 that caused these errors

@voxpelli voxpelli mentioned this issue Jan 29, 2024
Merged
voxpelli added a commit that referenced this issue Jan 29, 2024
@voxpelli
Fix MergeDeep in TypeScript 5.4
ee12b05 
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 voxpelli mentioned this issue Jan 29, 2024
Closed
@sindresorhus sindresorhus mentioned this issue Jan 30, 2024
Merged
@voxpelli voxpelli mentioned this issue Jan 31, 2024
Closed
@SantoJambit SantoJambit mentioned this issue Mar 8, 2024
Closed
2 tasks
BenoitZugmeyer added a commit to DataDog/browser-sdk that referenced this issue Mar 12, 2024
@BenoitZugmeyer
fix a typescript 5.4.2 regression causing typechecking to fail
bfa8824 
See:
* sindresorhus/type-fest#784
* mantinedev/mantine#5891
BenoitZugmeyer added a commit to DataDog/browser-sdk that referenced this issue Mar 12, 2024
@renovate @BenoitZugmeyer
👷 Update all non-major dependencies (#2637)
2e44e2e 
* 👷 Update all non-major dependencies

* fix a typescript 5.4.2 regression causing typechecking to fail

See:
* sindresorhus/type-fest#784
* mantinedev/mantine#5891

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Benoît Zugmeyer <benoit.zugmeyer@datadoghq.com>
zburke added a commit to folio-org/stripes-types that referenced this issue Mar 13, 2024
@zburke
STTYPES-11 bump type-fest to ^4.12.0
d3529a7 
Bump `type-fest` to `^4.12.0` for typescript v5.4 compatibility,
avoiding sindresorhus/type-fest#784

Refs STTYPES-11
@zburke zburke mentioned this issue Mar 13, 2024
Merged
zburke added a commit to folio-org/stripes-types that referenced this issue Mar 13, 2024
@zburke
STTYPES-11 bump type-fest to ^4.12.0 (#58)
988fda4 
Bump `type-fest` to `^4.12.0` for typescript v5.4 compatibility,
avoiding sindresorhus/type-fest#784

Refs STTYPES-11
@voxpelli voxpelli mentioned this issue Mar 13, 2024
Closed
kant2002 added a commit to kant2002/be-types that referenced this issue Mar 17, 2024
@kant2002
Add support for TypeScript 5.4
ad21211 
Fixes sindresorhus/type-fest#784 manifestation.
@kant2002 kant2002 mentioned this issue Mar 17, 2024
Open
kant2002 added a commit to kant2002/be-pkg-test that referenced this issue Mar 17, 2024
@kant2002
Add support for TypeScript 5.4
05b19e5 
Fixes sindresorhus/type-fest#784 manifestation.
@kant2002 kant2002 mentioned this issue Mar 17, 2024
Open
@jdbruijn jdbruijn mentioned this issue May 9, 2024
Closed
@yongkanm yongkanm mentioned this issue Jun 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@voxpelli @jakebailey