supabase / postgres-meta

A RESTful API for managing your Postgres. Fetch tables, add roles, and run queries
https://supabase.com
Apache License 2.0
911 stars 121 forks source link

change(typegen: TS): Export type, not interface #689

Closed isaacharrisholt closed 8 months ago

isaacharrisholt commented 9 months ago

What kind of change does this PR introduce?

Minor change

What is the current behavior?

Currently, the TypeScript type generation exports the Database type as an interface. However, this is somewhat problematic in that TypeScript interfaces don't implicitly define an index signature for their keys.

What do I mean by this?

Consider the following example:

type Pokemon = {
  name: string
  type: string | [string, string]
}

type Pokedex = Record<string, Pokemon>

interface MyPokedex {  // Note: interface!
  bulbasaur: Pokemon
  charmander: Pokemon
  squirtle: Pokemon
}

function getPokemon<T extends Pokedex>(pokedex: T): (keyof T)[] {
  return Object.keys(pokedex)
}

I would expect to be able to call getPokemon with MyPokedex as the type parameter. However, this results in a type error:

Type 'MyPokedex' does not satisfy the constraint 'Pokedex'.
Index signature for type 'string' is missing in type 'MyPokedex'.

What's the problem?

This means that if a library author wants to create a generic function where the expected type parameter is the user's Database type, there's no way for them to create a constraint.

For example, isaacharrisholt/supawright uses the user's Database to accurately type input and output types for some of its functions (much like supabase-js). It uses the following code to constrain the type parameter passed into the withSupawright function:

import type { GenericSchema } from '@supabase/supabase-js/dist/module/lib/types'

export type GenericDatabase = Record<string, GenericSchema>

(yes, I'm stealing from Supabase, but it works)

However, if I try to have withSupawright<Database extends GenericDatabase>() and then call it with the exported Database interface, I get the error mentioned above:

Type 'Database' does not satisfy the constraint 'GenericDatabase'.
Index signature for type 'string' is missing in type 'Database'.

Changing the exported interface to instead be a type resolves this.

We've been running the following...

types:
    pnpm supabase gen types typescript --local | \
    sed 's/export interface Database {/export type Database = {/' \
    > src/types/database.ts

...in production for a while now, and it has no negative impact on the Supabase client, and doing SupabaseClient<Database> still works as expected.

It would make Supabase extension libraries like this one much easier to write, and I can't think of any downsides to this change.

I might be missing something huge, in which case, let me know.

What is the new behavior?

The TypeScript type generator returns a type, not an interface.

soedirgo commented 8 months ago

Thanks @isaacharrisholt! There's little reason to use interface now that I think about it, so unless this breaks existing projects this change LGTM.

kamilogorek commented 8 months ago

It can break if:

So in theory it is a breaking change, but at the same time, it's: a) generated code b) very low change of happening

soedirgo commented 8 months ago

In this case I think it's worth the risk - we can reconsider if we get complaints about it.