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

Update Type Implementation (postgrest-js side) #152

Closed
wants to merge 5 commits into from
Closed

Update Type Implementation (postgrest-js side) #152

wants to merge 5 commits into from

Conversation

beeequeue
Copy link

@beeequeue beeequeue commented Feb 1, 2021

What kind of change does this PR introduce?

A breaking type update, which allows easier and stricter type checking. This is a combined update for this package and supabase-js.

supabase/supabase-js#125

What is the current behavior?

  • You have to pass the object you're working on into .from<User>('users') every time you use it.
  • .select() does not have any intellisense help.

What is the new behavior?

You get intellisense suggestions in .select().


You now pass the schema into the PostgrestClient, and let the types handle the rest from there.

The schema type is based on what openapi-typescript generates, as recommended in the supabase guide (which also has to be updated as the tool has renamed).

type User = {
  username: string
  data: object | null
  age_range: string | null
  status: 'ONLINE' | 'OFFLINE'
  catchphrase: 'string' | null
}

type Schema = {
  users: User
}

const supabase = new PostgrestClient<Schema>(REST_URL)

// Incorrect table, column names will result in type errors
typedClient.from('users').select('username').eq('username', 'supabot')

// Error
typedClient.from('uesrs').select('username').eq('username', 'wooo')
typedClient.from('users').select('username').eq('not_in_table', 'wooo')

Additional context

Not passing in a type still works as before, with any names working and returning any.

I tried very hard to not make this a breaking change, by still allowing .from<Object>(), but it's sadly impossible without some crazy workarounds. 😢

This package will have to be released before supabase-js so #123 can be updated to use the new version.

Comment on lines +120 to +121
users[0].username
users[0].somethingElse
Copy link
Author

Choose a reason for hiding this comment

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

These are for some reason nullable, but I can't find what in the code actually does it... They weren't nullable before and they aren't in other tests..

Comment on lines +35 to +37
if (Array.isArray(columns)) {
columns = columns.join(',')
}
Copy link
Author

Choose a reason for hiding this comment

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

This handles the array select input

@@ -21,7 +21,7 @@ export default class PostgrestQueryBuilder<T> extends PostgrestBuilder<T> {
* @param count Count algorithm to use to count rows in a table.
*/
select(
columns = '*',
columns: '*' | string | keyof T | Array<keyof T> = '*',
Copy link
Author

Choose a reason for hiding this comment

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

This will show '*', and the column names as suggestions, while still allowing a comma-separated string

Copy link
Author

@beeequeue beeequeue May 11, 2021

Choose a reason for hiding this comment

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

If this is going to be a part of a breaking change, I would recommend removing string (and therefore comma separated strings) as an option - IIRC it will then error if you give it an unknown column if a table type is passed in, and will result in string if no custom table type is passed.

columns: '*' | keyof T | Array<keyof T> = '*',

@jonlambert
Copy link

Went digging looking for how to type select() calls and came across this PR 🙂 Sorry to be 'that guy' - any idea of the state of this?

@kiwicopple
Copy link
Member

We're working on this as part of the Supabase v2 release (supabase/supabase-js#170), but that will be after Launch Week (next week). We'll do some project planning in August and should have a better idea of dates then @jonlambert

@jonlambert
Copy link

Brilliant, thanks @kiwicopple. I'll keep an eye out for that release.

@beeequeue
Copy link
Author

Guess I can close these now.

@beeequeue beeequeue closed this Sep 6, 2022
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.

3 participants