feat(types): setup postgrest-13 types inference - #618
Conversation
| ? ClientOptions['postgrestVersion'] extends 13 // Spread over an many-to-many relationship, turn all the result fields into arrays | ||
| ? ProcessManyToManySpreadNodeResult<Result> |
There was a problem hiding this comment.
note
Here is what the conditionality for the inference can look like.
| rest13: | ||
| image: postgrest/postgrest:v13.0.0 | ||
| ports: | ||
| - '3001:3000' | ||
| environment: | ||
| PGRST_DB_URI: postgres://postgres:postgres@db:5432/postgres | ||
| PGRST_DB_SCHEMAS: public,personal | ||
| PGRST_DB_EXTRA_SEARCH_PATH: extensions | ||
| PGRST_DB_ANON_ROLE: postgres | ||
| PGRST_DB_PLAN_ENABLED: 1 | ||
| PGRST_DB_TX_END: commit-allow-override | ||
| PGRST_DB_AGGREGATES_ENABLED: true | ||
| depends_on: | ||
| - db | ||
| rest12: | ||
| image: postgrest/postgrest:v12.2.0 |
There was a problem hiding this comment.
note
It is also possible to check if all the current tests and the new version ones are matching by upgrading the rest12 container to postgrest:v13. I did so for testing, and except from the Spread on Many feature, everything was behaving the same, with the only difference being the returned error message within some of the inline snapshots.
Co-authored-by: Steve Chavez <stevechavezast@gmail.com>
Allows loosy matching over the version minor
|
Coverage complain for no reason there. Edit: Exclude |
| export default abstract class PostgrestBuilder<Result, ThrowOnError extends boolean = false> | ||
| implements | ||
| export default abstract class PostgrestBuilder< | ||
| ClientOptions extends ClientServerOptions, |
There was a problem hiding this comment.
Won't block the PR based on this but I have a hunch that adding new type parameters at the start/middle of the parameter list will break some 3rd party lib relying on the typings
There was a problem hiding this comment.
Hum that's true, I've looked for usage on github: https://github.com/search?q=language%3ATypeScript++PostgrestBuilder%3C&type=code&p=3
Most of them seems to be forks though. Maybe we could pass this into a new major version though.
Or maybe in the changelog we can give a fix solution for compatibility:
import type { PostgrestBuilder as BasePostgrestBuilder } from '@supabase/posgrest-js';
type PostgrestBuilder<R, TE extends boolean = false> = BasePostgrestBuilder<{}, R, TE>Didn't tested it but this should allow to use the PostgrestBuilder type as before 🤔
I think I remember having trouble pushing this as a last argument because then, it required to always also define the ThrowOnError param which has default set.
|
🎉 This PR is included in version 1.20.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
What kind of change does this PR introduce?
This lay out the ground basis to start introducing new postgrest-13 features to the client library.
new PostgrestClient(DB_URL, { postgrestVersion: 12 | 13 }>allowing the types inference to pick in and choose the right result based on the available features for this major version.__internal_supabaseschema.