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

feat(types): export more PostgrestResponse, PostgrestBuilder and PostgrestClient types from postgrest-js #1357

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maximilian-hammerl
Copy link
Contributor

What kind of change does this PR introduce?

Export more PostgrestResponse, PostgrestBuilder and PostgrestClient types from postgrest-js

As the return type of, among others, the filter methods of the PostgrestFilterBuilder class, the types are significant if one wants to type the response when calling the filter methods.

What is the current behavior?

Currently, one has to have postgrest-js as a dependency just for the PostgrestResponse, PostgrestBuilder and PostgrestClient types.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 12825599938

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 67.805%

Totals Coverage Status
Change from base Build 12823066490: 0.0%
Covered Lines: 101
Relevant Lines: 130

💛 - Coveralls

@maximilian-hammerl
Copy link
Contributor Author

Hi @avallete, anything missing for this PR to get approved and merged? Anything I can help with?

@maximilian-hammerl maximilian-hammerl changed the title Export more PostgrestResponse, PostgrestBuilder and PostgrestClient types from postgrest-js feat(types): export more PostgrestResponse, PostgrestBuilder and PostgrestClient types from postgrest-js Jan 24, 2025
@avallete
Copy link
Member

avallete commented Jan 24, 2025

I'm not too sure we want to expose these types for external use.

Users might feel encouraged to build on top of them when using supabase-js and expect their behavior to remain stable. However, since these types are an internal implementation detail, we could break or remove them entirely in the future. This would force us to make major releases more frequently, as fixing behaviors could introduce breaking changes in the types.

@soedirgo, what do you think?

If we’re okay with more frequent breaking changes/major releases, then I guess it’s fine to go ahead.

Alternatively, we can continue require keeping postgres-js in the dev dependencies (as it is now) if users want to rely on its types for building code. That might be the safer approach.

@soedirgo
Copy link
Member

Yeah, these types are internal implementation detail as far as supabase-js is concerned, so we don't want users to be dependent on their stability.

For your use case, you can consider using TypeScript's ReturnType instead - I don't foresee that causing issues if we change the return types of filter methods etc.

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

Successfully merging this pull request may close these issues.

4 participants