-
Notifications
You must be signed in to change notification settings - Fork 160
Fix: Enable $selected access in orderBy after fn.select #1183
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
Conversation
The orderBy method was only checking for regular select clause
(this.query.select) to determine if $selected should be exposed.
When using fn.select, the callback is stored in fnSelect instead,
so $selected was not available in orderBy callbacks.
This change updates orderBy to check for both select and fnSelect,
enabling queries like:
```ts
q.from({ user: usersCollection })
.fn.select((row) => ({ name: row.user.name, salary: row.user.salary }))
.orderBy(({ $selected }) => $selected.salary)
```
https://claude.ai/code/session_01EPRCG2K8348FMNWzjPiacC
🦋 Changeset detectedLatest commit: 37725b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
More templates
@tanstack/angular-db
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: +9 B (+0.01%) Total Size: 90.9 kB
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.7 kB ℹ️ View Unchanged
|
Also adds tests for orderBy with table refs after fn.select. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The having method fix was made for consistency but couldn't be verified with a passing test. Keeping only the orderBy fix which has comprehensive test coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@KyleAMathews The |
- Added fnSelect check to having method for $selected access - Added test for fn.having with fn.select (no groupBy) - Note: fn.select + groupBy combination requires further work Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks for the feedback @kevin-dp! I've added What works now:
Limitation found: This happens because:
Supporting
For now, users should continue using regular Let me know if you'd like me to pursue the |
kevin-dp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this and follow up on the fn.select + having in a separate follow up PR.
|
Posted issue to #1189 |
Summary
Fixed
$selectednamespace availability inorderBy(),having(), andfn.having()clauses when using functional select (fn.select()). Users can now order and filter by computed/derived fields from functional selects.Root Cause
The query builder only checked for
this.query.selectwhen deciding whether to expose the$selectednamespace inorderByandhavingcallbacks. It did not account forthis.query.fnSelect(functional select), so users couldn't reference$selectedfields after usingfn.select().Approach
Added
|| this.query.fnSelectto the condition in bothorderBy()andhaving()methods:Key Invariants
$selectedis available inorderBy,having, andfn.havingwhen either.select()orfn.select()has been called$selected(both namespaces work together)fn.select()Limitations
fn.select+groupBycombination is not fully supported (returns only group keys, not computed fields).select()with.having()or.fn.having()Verification
Files Changed
packages/db/src/query/builder/index.tsfnSelectcheck toorderByandhavingmethodspackages/db/tests/query/functional-variants.test.tsfn.selectwithorderByandfn.havingpackages/db/tests/query/functional-variants.test-d.ts$selectedtype inferencehttps://claude.ai/code/session_01EPRCG2K8348FMNWzjPiacC