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

Inconsistent Typing for Joined Field in Supabase #546

Closed
2 tasks done
cptcrunchy opened this issue Jun 17, 2024 · 20 comments
Closed
2 tasks done

Inconsistent Typing for Joined Field in Supabase #546

cptcrunchy opened this issue Jun 17, 2024 · 20 comments
Labels
bug Something isn't working

Comments

@cptcrunchy
Copy link

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

I have added a link to a REPL, HERE along with a screenshot.
The example code expects the created_by field in the query result to be a single object containing employee details (id and full_name). However, the type definition ({ id: any; full_name: any; }[]) suggests it's an array of employees. This mismatch causes a typing error when working with the data.

Why it happens:

The !report_created_by_fkey syntax likely defines a relationship between tables, but the return type might not be explicitly set to a single object.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Go to The REPL link
  2. Enter your Project URL and ANON key into the createClient instance.
  3. Create the table schemas provided in $lib/db
  4. When hovering over data the return type def is an array of objects.

Expected behavior

The result type def should be a single object for the field created_by.

Screenshots

If applicable, add screenshots to help explain your problem.
Screenshot 2024-06-17 at 15 57 57

System information

  • OS: macOS
  • Version of supabase-js: 2.42.7
  • Version of Node.js: 18.20.3

Additional context

Add any other context about the problem here.

@cptcrunchy cptcrunchy added the bug Something isn't working label Jun 17, 2024
@mansueli mansueli transferred this issue from supabase/supabase Jun 26, 2024
@jdgamble555
Copy link

@soedirgo - This is the same issue opened MANY times and still needs to be fixed.

We have 8 different repos all for the same problem. JOINS do not have the correct typing. This needs to be moved up the ladder as a PRIORITY.

J

@jdgamble555
Copy link

@soedirgo - Can we please get this looked at? Every single join is effected, there are hundreds of complaints. Please raise this issue to the top of the list!

Thanks,

J

@FernandoI7
Copy link

+1

@osilviotti
Copy link

osilviotti commented Aug 8, 2024

I've also encountered this recently.

In case it's helpful, querying the table directly gave the correct type for the relation (e.g. { id: string; } | null), but calling an RPC function that returned a SETOF the same table gave the incorrect type for the same relation (e.g. { id: string }[]).

The field types in the Supabase generated Database are identical between the table Row and the function Returns, but the PostgrestFilterBuilder isn't passed the Relationships in the case of the RPC function so I suspect that might be why.

EDIT: For now, I'm using a type to override what's being returned from the library. It might only be useful in my specific instance, but thought I'd drop it here in case it helps anyone else.

const getTasks = () => {
  return supabase
    .rpc('get_tasks')
    .select('id, assignee:users(name), location:locations(name)');
};

type Task = QueryData<ReturnType<typeof getTasks>>[number];
// { id: string; assignee: { name: string; }[], location: { name: string; }[]; }

type PostgrestRelationStopgap<
  Type extends Record<string, any>,
  Keys extends keyof Type,
  Nullable extends keyof Type | boolean = false,
> =
  & Omit<Type, Keys>
  & {
    [Key in Keys]:
      | (Nullable extends true ? null
        : Key extends Nullable ? null
        : never)
      | Pick<Type, Key>[Key][number];
  };
  
type TaskA = PostgrestRelationStopgap<Task, "assignee" | "location">;
// { id: string; assignee: { name: string; }, location: { name: string; }; }

type TaskB = PostgrestRelationStopgap<Task, "assignee" | "location", true>;
// { id: string; assignee: { name: string; } | null, location: { name: string; } | null; }

type TaskC = PostgrestRelationStopgap<Task, "assignee" | "location", "location">;
// { id: string; assignee: { name: string; }, location: { name: string; } | null; }

@jdgamble555
Copy link

@osilviotti - RPC functions are a different problem and should be in a new post. I don't want the Supabase staff to get off topic from the issue at hand.

I don't know your use case, but most MOST join cases do not work as expected. You can scroll through the issues listed above.

J

@alfredomariamilano
Copy link

It's not ideal, but for the time being, I'm fixing it like so:

supabase.from('posts').select('author:profiles!user_id(username)' as 'author:profiles!screens_user_id_fkey(username)')

Types are inferred correctly if you use the foreign key constraint's name, which should help solve this.

@clement-faure
Copy link

Same issue here following documentation. I've been able to find a workaround using this to populate one to one relationship.

created_by: employees !inner(*)

@soedirgo
Copy link
Member

soedirgo commented Sep 10, 2024

I couldn't reproduce the issue in this repo - here's the types I'm getting:

Screenshot 2024-09-10 at 6 26 09 PM

If anyone can reproduce the issue, can you share the contents of your generated types (database.types.ts)?

@alfredomariamilano
Copy link

I couldn't reproduce the issue in this repo - here's the types I'm getting:

Screenshot 2024-09-10 at 6 26 09 PM If anyone can reproduce the issue, can you share the contents of your generated types (`database.types.ts`)?

The types work if you use the foreignKeyName, but the request will fail because it expects the column's name.
In my example above ('author:profiles!user_id(username)' as 'author:profiles!screens_user_id_fkey(username)'), the column holding the foreign key is user_id, but the name of the foreign key is screens_user_id_fkey. postgrest expects the name of the column to run the actual query, but it expects the name of the foreign key for the types.

@jdgamble555
Copy link

@soedirgo - I have created an example REPO with all the problems I can think of right now.

// DOES NOT WORK, 'users' is possibly null, clearly there is a required FK
const test1 = await supabase.from('posts').select('users(*)')

// DOES NOT WORK, 'user_id' is empty array
const test2 = await supabase.from('posts').select('user_id(*)');

// DOES NOT WORK, 'user' is empty array
const test3 = await supabase.from('posts').select('user:user_id(*)');

// DOES NOT WORK, 'users' is array instead of single
const test4 = await supabase.from('posts').select('users!user_id(*)');

// DOES NOT WORK, 'user' is array instead of single
const test5 = await supabase.from('posts').select('user:users!user_id(*)');

See repo.

J

@avallete
Copy link
Member

Hey there,

I'm currently working on this issue in PR #554. I've added more tests to ensure better consistency between runtime behavior and type inference results. Hopefully, this will cover more cases, although I'm not certain it will handle all of them—there are quite a few edge cases to consider.

Additionally, I wonder if it might be worth adding more "hinting" for type inference in the database introspection results. For example, in cases where the result of a foreign key (FK) is nullable when it shouldn't be, it might be easier to resolve these cases by providing this information directly inside the "relationship" object. This approach could potentially avoid a convoluted type search across relations and table columns to check if the value of the holding column can be null or not.

So far, I still have six divergences between types and runtime values in the test scenarios I’ve set up. Most of these involve relation results being possibly null when they shouldn't be, and some are due to "relation aliasing." Depending on whether the user uses a direct foreign key name, a column name, or the destination table (in cases where there is a single relation between table1 and table2), the parser fails to determine the correct result type.

@soedirgo, I believe most of these issues could be addressed by slightly augmenting the data we pull during introspection (e.g., handling the nullable issue). Would this be something worth considering? It would probably require users to rebuild their types and could result in a breaking change between pre- and post-version for the query parser. What do you think?

@avallete
Copy link
Member

Hey everyone !

We've reworked the result inference in postgrest-js to address the typing issues mentioned above. The changes are available in a canary release, and we're actively seeking your feedback.

To test it out, update your supabase-js to version 2.46.0-rc.1 (which includes postgrest-js 1.17.0-rc.1):

npm install supabase-js@2.46.0-rc.1

Please let us know if you encounter any issues, especially with retro-compatibility, so we can finalize it for the next release.

@alfredomariamilano
Copy link

Hey everyone !

We've reworked the result inference in postgrest-js to address the typing issues mentioned above. The changes are available in a canary release, and we're actively seeking your feedback.

To test it out, update your supabase-js to version 2.46.0-rc.1 (which includes postgrest-js 1.17.0-rc.1):

npm install supabase-js@2.46.0-rc.1

Please let us know if you encounter any issues, especially with retro-compatibility, so we can finalize it for the next release.

Hi @avallete! Sorry for the late reply and thanks for the work on the RC. I've tested it in my project for its use-cases and it works like a charm and infers the correct types!

@avallete
Copy link
Member

The fix has been released in supabase-js v2.46.0. I'm closing this issue, but feel free to reopen or open a new issue if you encounter any further errors.

@steviehol
Copy link

steviehol commented Nov 1, 2024

Can this be reopened. Im still facing this issue and looking at the changelog this was reverted.

@avallete
Copy link
Member

avallete commented Nov 1, 2024

Can this be reopened. Im still facing this issue and looking at the changelog this was reverted.

Yes it has been reverted as it caused unexpected regressions with previous version.

FYI: We are working on fixing those and carry on with the patch. @StevieBoyWonder in the meantime if there is no regression on your codebase you can use 2.46.0 which should fix this issue for you.

@avallete avallete reopened this Nov 1, 2024
@jdgamble555
Copy link

@avallete - Are you guys fixing this still? Everything was working fine before you reverted it...

@avallete
Copy link
Member

@avallete - Are you guys fixing this still? Everything was working fine before you reverted it...

Hey ! Thanks for the feedback and yes we are.

We think that we managed to fix most of the regressions that had been reported so this should land pretty soon.

Meanwhile you can use the new types inference version through the "rc" releases of supabase-js

@avallete
Copy link
Member

@jdgamble555 Happy to tell you that we just released v2.47.8 that include the typing inference rework which should fix the inconsistents typing on joined fields.

@jdgamble555
Copy link

@avallete - Could you look at this, I believe this broke RPC function typing, which should be an easy fix I would imagine.

#602

J

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants