Skip to content

Inferred types for nullable columns after not null checks should be non-nullable #1360

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

Open
2 tasks done
chesterhow opened this issue Jan 24, 2025 · 7 comments
Open
2 tasks done
Labels
bug Something isn't working

Comments

@chesterhow
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

For nullable columns, even if the query includes a not null check (e.g. .not("column_name", "is", null)), the inferred return type for that column is still nullable.

To Reproduce

  1. Create a blog_posts table with a nullable column, published_at.
create table blog_posts(
  id           uuid        primary key default uuid_generate_v4(),
  title        text        not null,
  created_at   timestamptz not null default now(),
  published_at timestamptz
);
  1. Query this table to retrieve all published blog posts.
const { data } = await supabase
  .from("blog_posts")
  .select("*")
  .not("published_at", "is", null);
  1. Observe that the published_at field in data is still typed as nullable.
const data: {
  id: string;
  title: string;
  created_at: string;
  published_at: string | null; // <-- why is this nullable?
}[] | null

Expected behavior

Inferred type for published_at should not be nullable.

System information

  • Version of supabase-js: 2.48.0
@chesterhow chesterhow added the bug Something isn't working label Jan 24, 2025
@avallete
Copy link
Member

Hi @chesterhow,

Thanks for bringing this up! It’s definitely an interesting idea, but I don’t see us implementing this anytime soon. Here’s why:

Implementing this would mean we’d need to replicate the full filtering logic of PostgREST within the TypeScript type system. Not only would that be pretty complex, but it could also lead to performance issues, especially when dealing with dynamic filters.

Another thing to consider is consistency. If we were to handle not null checks at the type level, you might expect similar behavior for other operators. For example:

const { data } = await supabase
  .from("blog_posts")
  .select("*")
  .eq("title", 'somevalue');

const data: {
  id: string;
  title: string; // <--- Should this now only allow "somevalue" as the type?
  created_at: string;
}[] | null

As you can see, this could quickly snowball into a maintenance challenge, and we’d need to ensure it works seamlessly across all operators and edge cases.

For now, we recommend handling these cases manually in your code to ensure type safety. We’ll keep this in mind as we continue improving the library, though!

@avallete avallete added wontfix This will not be worked on and removed wontfix This will not be worked on labels Jan 24, 2025
@chesterhow
Copy link
Author

Hey @avallete, thanks for sharing more about the constraints of implementing this! 👀

May I know what is the recommended approach to deal with typing limitations like these? Should I override types using .returns<T>(), manually write type guards, or is there some other way?

@avallete
Copy link
Member

avallete commented Feb 3, 2025

Hey @chesterhow !

Indeed using .overrideTypes or an explicit cast are two valid ways to deal with this depending on your preferences.

@franckzi

This comment has been minimized.

@raky291
Copy link

raky291 commented Mar 30, 2025

When I saw it, I thought it was a bug, it would be nice to mention it in the documentation 👀 mention that you can use .returns or an explicit cast

@avallete
Copy link
Member

avallete commented Mar 30, 2025

When I saw it, I thought it was a bug, it would be nice to mention it in the documentation 👀 mention that you can use .returns or an explicit cast

Yes, we added a new method mentioned here: https://supabase.com/docs/guides/api/rest/generating-types#using-typescript-type-definitions

And in the reference docs here:

https://supabase.com/docs/reference/javascript/db-returns

overrideTypes allows you to only override what you care about while keeping most of the type inferred from the query string. But that might need to be more prominent in our docs thanks for your feedback.

However I want to mention that this is classified as a bug, we might still have this fixed in the future. But we still need to think it through. That's why the issue stays open so it can be discussed over.

@raky291
Copy link

raky291 commented Mar 31, 2025

@avallete Thanks 😄

My 2c here is that maybe creating a new method .notNull("column") to handle that specific case without affecting other filters. 🤔

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

4 participants