Skip to content

fix(typegen): typescript: deprecate Json type in favour of unknown #750

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Conversation

isaacharrisholt
Copy link
Contributor

@isaacharrisholt isaacharrisholt commented Mar 21, 2024

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

#676

What is the new behavior?

Json has been marked as deprecated and is now just equivalent to unknown.

FIxes #676

@isaacharrisholt isaacharrisholt requested review from a team as code owners March 21, 2024 10:09
@avallete
Copy link
Member

avallete commented Mar 29, 2025

Hi there !

Thank you for your contribution !

This looks great, and would also help when overriding the Database default Json types for a specific ones using the type-fest, MergeDeep utilities like recommended in the docs eg:

export type CustomUserDataType = {
  foo: string
  bar: {
    baz: number
  }
  en: 'ONE' | 'TWO' | 'THREE'
  record: Record<string,  string> | null
  recordNumber: Record<number, string> | null
}

export type Database = MergeDeep<
  GeneratedDatabase,
  {
    public: {
      Tables: {
        users: {
          Row: {
            data: CustomUserDataType | null
          }
          Insert: {
            data?: CustomUserDataType | null
          }
          Update: {
            data?: CustomUserDataType | null
          }
        }
      }
    }
  }
>

Such override will result in a non-clean merge object (some union will happen) if they type merged to is Json, but will work just fine with unknown.

However, I would keep this as a flagged feature change for now, similar to the detect_one_to_one_relationships option, that we can then turn-on by default.

It'll allow us to deploy the change gradually and still offer users for who it might be a breaking change to gradually opt-in and refactor when they're ready. Without breaking their types definitions.

Maybe something like json_as_unknown.

Edit:

Another thing to consider is that the postgrest-js type inference also rely on a Json type for the "json accessor typing" fonctionnality: https://github.com/supabase/postgrest-js/blob/956ed181147b3477daf81311c374c643323eab76/src/select-query-parser/types.ts#L16-L24

This will probably need to be changed as well so for consistency with this change 🤔

So this might end up being more of a breaking change than expected. The only way I can think of doing this without breaking change is by refactoring the types inference to have a dynamic "Json" type instead of a static one, but it might not be worth it though if that's not the way we want to go moving forward.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate types: "Json" type is incompatible with other types
2 participants