fix(postgrest): add typed column inference for order() with referencedTable#2445
Open
7vignesh wants to merge 1 commit into
Open
fix(postgrest): add typed column inference for order() with referencedTable#24457vignesh wants to merge 1 commit into
7vignesh wants to merge 1 commit into
Conversation
…dTable When referencedTable or foreignTable is a known table/view, the column param is now constrained to that table's Row. Provides autocomplete and compile-time error checking. Closes supabase#971
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates Postgrest-js TypeScript typings so .order() can infer valid column names when ordering via a referencedTable (and the deprecated foreignTable) based on the schema.
Changes:
- Add
TablesAndViews-based overloads for.order()to type column names forreferencedTable. - Add matching typed overloads for deprecated
foreignTable. - Extend d.ts tests to cover typed ordering on referenced tables and expected type errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/core/postgrest-js/src/PostgrestTransformBuilder.ts | Adds schema-aware .order() overloads for referencedTable/foreignTable to type referenced-table columns. |
| packages/core/postgrest-js/test/index.test-d.ts | Adds type-level tests verifying typed referenced-table ordering and invalid-column failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+148
to
+156
| order<ReferencedTable extends string>( | ||
| column: string, | ||
| options?: { ascending?: boolean; nullsFirst?: boolean; referencedTable?: string } | ||
| options?: { | ||
| ascending?: boolean | ||
| nullsFirst?: boolean | ||
| referencedTable?: ReferencedTable extends keyof TablesAndViews<Schema> | ||
| ? never | ||
| : ReferencedTable | ||
| } |
Comment on lines
+178
to
+184
| order<ReferencedTable extends string>( | ||
| column: string, | ||
| options?: { ascending?: boolean; nullsFirst?: boolean; foreignTable?: string } | ||
| options?: { | ||
| ascending?: boolean | ||
| nullsFirst?: boolean | ||
| foreignTable?: ReferencedTable extends keyof TablesAndViews<Schema> ? never : ReferencedTable | ||
| } |
Comment on lines
+141
to
+147
| order< | ||
| ReferencedTable extends string & keyof TablesAndViews<Schema>, | ||
| ColumnName extends string & keyof TablesAndViews<Schema>[ReferencedTable]['Row'], | ||
| >( | ||
| column: ColumnName, | ||
| options: { ascending?: boolean; nullsFirst?: boolean; referencedTable: ReferencedTable } | ||
| ): this |
Comment on lines
+168
to
+174
| order< | ||
| ReferencedTable extends string & keyof TablesAndViews<Schema>, | ||
| ColumnName extends string & keyof TablesAndViews<Schema>[ReferencedTable]['Row'], | ||
| >( | ||
| column: ColumnName, | ||
| options: { ascending?: boolean; nullsFirst?: boolean; foreignTable: ReferencedTable } | ||
| ): this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What changed?
Added new typed overloads to the
order()method inPostgrestTransformBuilderthat resolve the referenced table's column names from theSchematype whenreferencedTable(or deprecatedforeignTable) is a known table/view.Overload resolution now works in 3 tiers:
referencedTable→ autocomplete shows columns from the parent table (unchanged)referencedTableis a known table/view → autocomplete shows columns from that table'sRow, invalid columns produce a compile-time errorreferencedTableis an unknown string → falls through tocolumn: string(unchanged, for dynamic/alias use cases)The catch-all overload uses a conditional
nevertype to prevent it from matching when the referenced table is known, forcing TypeScript to use the typed overload instead.Why was this change needed?
When users passed
referencedTable: 'messages'to.order(), TypeScript offered no autocomplete for the referenced table's columns and silently accepted invalid column names. This caused runtime errors that could have been caught at compile time.This is the TypeScript typing portion of the issue — runtime behavior was already correct (clarified by maintainers in the issue thread).
Closes #971
📸 Screenshots/Examples
After: type error on invalid column for referenced table
After: valid column passes without error
Breaking changes
The catch-all
stringoverload still exists for dynamic table names or unknown aliases, so existing code that passes arbitrary strings continues to compile.Checklist
fix(postgrest): add typed column inference for order() with referencedTablepnpm nx formatto ensure consistent code formattingtest/index.test-d.ts)Additional notes
TablesAndViewsfrom the existing select-query-parser types no new type utilities were introduced.order()is untouched.postgrest-jsand downstreamsupabase-js.