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

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

Open
2 tasks done
chesterhow opened this issue Jan 24, 2025 · 4 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 .returns or an explicit cast are two valid ways to deal with this depending on your preferences.

@franckzi

This comment has been minimized.

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

3 participants