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(types): type instantiation is excessively deep and possibly infinite #597

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

avallete
Copy link
Member

@avallete avallete commented Jan 20, 2025

What kind of change does this PR introduce?

Bug fix a case on typescript > 4.5.5 where using MergeDeep override lead to an error of Type instantiation is excessively deep and possibly infinite. Use a trick to "force typescript to infer final type" instead of doing a shallow inspection and warning about possible infinite recursion.

This is a bit black magic'ish but it fix the issue on the reproduction case 😅

Additional context

Related: supabase/supabase-js#1354

When using MergeDeep type-fest even with simple structures we get this error
For typescript > 4.5.5 this trick fix the issue by forcing TS to infer the value
And reuse it instead of computing multiple time
Comment on lines 76 to 80
value: ResolveFilterValue<Schema, Row, ColumnName> extends infer ResolvedFilterValue
? ResolvedFilterValue extends never
? NonNullable<unknown>
: NonNullable<ResolvedFilterValue>
: never
Copy link
Member Author

Choose a reason for hiding this comment

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

The actual tricks is to use the infer and never to force typescript to infer the right type and not stop at "possibly infinite recursive".

@avallete avallete changed the title Fix/issue 1354 type instanciation error fix(types): type instantiation is excessively deep and possibly infinite Jan 20, 2025
Comment on lines +276 to +282
values: ReadonlyArray<
ResolveFilterValue<Schema, Row, ColumnName> extends infer ResolvedFilterValue
? ResolvedFilterValue extends never
? unknown[]
: ResolvedFilterValue
: never
>
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we need the infer within the ReadonlyArray to have the proper resulting type, otherwise we'll have an invalid union.

Comment on lines +1834 to +1836
// @ts-expect-error should not be able to eq 'id' since the column does not
// exist
.eq('id', 2)
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this test should never had passed because the query actually is made to return an error. And id column doesn't exist in users table.

We can make it pass tough by changing the never into an unknown here:

https://github.com/supabase/postgrest-js/pull/597/files#diff-8c09e973443260962548f07f6714d574185bce9a909a6d818691993e5a4253e5L43

@avallete avallete marked this pull request as ready for review January 20, 2025 14:44
@Eigilak
Copy link

Eigilak commented Jan 22, 2025

Not All Heroes Wear Capes 🫡

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.

2 participants