Skip to content

Commit eeb5321

Browse files
kevin-dpclaudeautofix-ci[bot]
authored
fix(db): use Ref<T, Nullable> for left join select refs instead of Ref<T> | undefined (#1262)
* test(db): add type tests for left join select ref direct property access Declarative select callback types currently use `Ref<T> | undefined` for left-joined tables, forcing optional chaining. Since the callback receives proxy refs that are always truthy, this is misleading and encourages runtime conditional patterns that silently fail. These tests assert the desired behavior: direct property access without optional chaining, with nullability reflected only in the result type. Co-Authored-By: Claude Opus 4.6 <[email protected]> * test(db): add right join and full join type tests for select ref direct access Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix(db): use Ref<T, Nullable> brand instead of Ref<T> | undefined for nullable join refs The declarative select() callback receives proxy objects that record property accesses. These proxies are always truthy, but nullable join sides were typed as Ref<T> | undefined, misleading users into using ?. and ?? operators that have no effect at runtime. This changes nullable join refs to Ref<T, true>, which allows direct property access while correctly producing T | undefined in the result type. Co-Authored-By: Claude Opus 4.6 <[email protected]> * ci: apply automated fixes * chore: add changeset for nullable join ref typing fix Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 41c0ea2 commit eeb5321

6 files changed

Lines changed: 274 additions & 157 deletions

File tree

‎.changeset/fix-nullable-join-select-ref-typing.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@tanstack/db': patch
3+
---
4+
5+
fix(db): use `Ref<T, Nullable>` brand instead of `Ref<T> | undefined` for nullable join refs in declarative select
6+
7+
The declarative `select()` callback receives proxy objects that record property accesses. These proxies are always truthy at build time, but nullable join sides (left/right/full) were typed as `Ref<T> | undefined`, misleading users into using `?.` and `??` operators that have no effect at runtime. Nullable join refs are now typed as `Ref<T, true>`, which allows direct property access without optional chaining while correctly producing `T | undefined` in the result type.

‎packages/db-collection-e2e/src/suites/joins.suite.ts‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
2424
eq(user.id, post.userId),
2525
)
2626
.select(({ user, post }) => ({
27-
id: post!.id,
27+
id: post.id,
2828
userName: user.name,
29-
postTitle: post!.title,
29+
postTitle: post.title,
3030
})),
3131
)
3232

@@ -53,12 +53,12 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
5353
.join({ post: postsCollection }, ({ user, post }) =>
5454
eq(user.id, post.userId),
5555
)
56-
.where(({ post }) => gt(post!.viewCount, 10))
56+
.where(({ post }) => gt(post.viewCount, 10))
5757
.select(({ user, post }) => ({
58-
id: post!.id,
58+
id: post.id,
5959
userName: user.name,
60-
postTitle: post!.title,
61-
viewCount: post!.viewCount,
60+
postTitle: post.title,
61+
viewCount: post.viewCount,
6262
})),
6363
)
6464

@@ -88,9 +88,9 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
8888
eq(user.id, post.userId),
8989
)
9090
.select(({ user, post }) => ({
91-
id: post!.id,
91+
id: post.id,
9292
userName: user.name,
93-
postTitle: post!.title,
93+
postTitle: post.title,
9494
})),
9595
)
9696

@@ -117,12 +117,12 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
117117
.join({ post: postsCollection }, ({ user, post }) =>
118118
eq(user.id, post.userId),
119119
)
120-
.orderBy(({ post }) => post!.viewCount, `desc`)
120+
.orderBy(({ post }) => post.viewCount, `desc`)
121121
.select(({ user, post }) => ({
122-
id: post!.id,
122+
id: post.id,
123123
userName: user.name,
124-
postTitle: post!.title,
125-
viewCount: post!.viewCount,
124+
postTitle: post.title,
125+
viewCount: post.viewCount,
126126
})),
127127
)
128128

@@ -149,7 +149,7 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
149149
for (let i = 1; i < results.length; i++) {
150150
const prevCount = results[i - 1]!.viewCount
151151
const currCount = results[i]!.viewCount
152-
expect(prevCount).toBeGreaterThanOrEqual(currCount)
152+
expect(prevCount!).toBeGreaterThanOrEqual(currCount!)
153153
}
154154

155155
await query.cleanup()
@@ -166,13 +166,13 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
166166
.join({ post: postsCollection }, ({ user, post }) =>
167167
eq(user.id, post.userId),
168168
)
169-
.orderBy(({ post }) => post!.id, `asc`)
169+
.orderBy(({ post }) => post.id, `asc`)
170170
.limit(10)
171171
.offset(5)
172172
.select(({ user, post }) => ({
173-
id: post!.id,
173+
id: post.id,
174174
userName: user.name,
175-
postTitle: post!.title,
175+
postTitle: post.title,
176176
})),
177177
)
178178

@@ -194,13 +194,13 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
194194
.from({ user: users })
195195
.join({ post: posts }, ({ user, post }) => eq(user.id, post.userId))
196196
.join({ comment: comments }, ({ post, comment }) =>
197-
eq(post!.id, comment.postId),
197+
eq(post.id, comment.postId),
198198
)
199199
.select(({ user, post, comment }) => ({
200-
id: comment!.id,
200+
id: comment.id,
201201
userName: user.name,
202-
postTitle: post!.title,
203-
commentText: comment!.text,
202+
postTitle: post.title,
203+
commentText: comment.text,
204204
})),
205205
)
206206

@@ -225,16 +225,16 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
225225
.from({ user: users })
226226
.where(({ user }) => eq(user.isActive, true))
227227
.join({ post: posts }, ({ user, post }) => eq(user.id, post.userId))
228-
.where(({ post }) => isNull(post!.deletedAt))
228+
.where(({ post }) => isNull(post.deletedAt))
229229
.join({ comment: comments }, ({ post, comment }) =>
230-
eq(post!.id, comment.postId),
230+
eq(post.id, comment.postId),
231231
)
232-
.where(({ comment }) => isNull(comment!.deletedAt))
232+
.where(({ comment }) => isNull(comment.deletedAt))
233233
.select(({ user, post, comment }) => ({
234-
id: comment!.id,
234+
id: comment.id,
235235
userName: user.name,
236-
postTitle: post!.title,
237-
commentText: comment!.text,
236+
postTitle: post.title,
237+
commentText: comment.text,
238238
})),
239239
)
240240

@@ -264,13 +264,13 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
264264
eq(user.id, post.userId),
265265
)
266266
.join({ comment: commentsOnDemand }, ({ post, comment }) =>
267-
eq(post!.id, comment.postId),
267+
eq(post.id, comment.postId),
268268
)
269269
.select(({ user, post, comment }) => ({
270-
id: comment!.id,
270+
id: comment.id,
271271
userName: user.name,
272-
postTitle: post!.title,
273-
commentText: comment!.text,
272+
postTitle: post.title,
273+
commentText: comment.text,
274274
})),
275275
)
276276

@@ -296,12 +296,12 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
296296
q
297297
.from({ user: users })
298298
.join({ post: posts }, ({ user, post }) => eq(user.id, post.userId))
299-
.where(({ post }) => gt(post!.viewCount, 50))
299+
.where(({ post }) => gt(post.viewCount, 50))
300300
.select(({ user, post }) => ({
301-
id: post!.id,
301+
id: post.id,
302302
userName: user.name,
303-
postTitle: post!.title,
304-
viewCount: post!.viewCount,
303+
postTitle: post.title,
304+
viewCount: post.viewCount,
305305
})),
306306
)
307307

@@ -326,10 +326,10 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
326326
.where(({ user }) => gt(user.age, 30))
327327
.join({ post: posts }, ({ user, post }) => eq(user.id, post.userId))
328328
.select(({ user, post }) => ({
329-
id: post!.id,
329+
id: post.id,
330330
userName: user.name,
331331
userAge: user.age,
332-
postTitle: post!.title,
332+
postTitle: post.title,
333333
})),
334334
)
335335

@@ -359,7 +359,7 @@ export function createJoinsTestSuite(getConfig: () => Promise<E2ETestConfig>) {
359359
.select(({ user, post }) => ({
360360
id: user.id,
361361
userName: user.name,
362-
postTitle: post!.title, // May be null for users without posts
362+
postTitle: post.title, // May be null for users without posts
363363
})),
364364
)
365365

‎packages/db/src/query/builder/types.ts‎

Lines changed: 64 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -227,18 +227,28 @@ export type ResultTypeFromSelect<TSelectObject> = WithoutRefBrand<
227227
Prettify<{
228228
[K in keyof TSelectObject]: NeedsExtraction<TSelectObject[K]> extends true
229229
? ExtractExpressionType<TSelectObject[K]>
230-
: TSelectObject[K] extends Ref<infer _T>
230+
: // Ref (full object ref or spread with RefBrand) - recursively process properties
231+
TSelectObject[K] extends Ref<infer _T>
231232
? ExtractRef<TSelectObject[K]>
232-
: TSelectObject[K] extends RefLeaf<infer T>
233-
? T
234-
: TSelectObject[K] extends RefLeaf<infer T> | undefined
233+
: // RefLeaf (simple property ref like user.name)
234+
TSelectObject[K] extends RefLeaf<infer T>
235+
? IsNullableRef<TSelectObject[K]> extends true
235236
? T | undefined
236-
: TSelectObject[K] extends RefLeaf<infer T> | null
237-
? T | null
238-
: TSelectObject[K] extends Ref<infer _T> | undefined
239-
? ExtractRef<TSelectObject[K]> | undefined
240-
: TSelectObject[K] extends Ref<infer _T> | null
241-
? ExtractRef<TSelectObject[K]> | null
237+
: T
238+
: // RefLeaf | undefined (schema-optional field)
239+
TSelectObject[K] extends RefLeaf<infer T> | undefined
240+
? T | undefined
241+
: // RefLeaf | null (schema-nullable field)
242+
TSelectObject[K] extends RefLeaf<infer T> | null
243+
? IsNullableRef<Exclude<TSelectObject[K], null>> extends true
244+
? T | null | undefined
245+
: T | null
246+
: // Ref | undefined (optional object-type schema field)
247+
TSelectObject[K] extends Ref<infer _T> | undefined
248+
? ExtractRef<Exclude<TSelectObject[K], undefined>> | undefined
249+
: // Ref | null (nullable object-type schema field)
250+
TSelectObject[K] extends Ref<infer _T> | null
251+
? ExtractRef<Exclude<TSelectObject[K], null>> | null
242252
: TSelectObject[K] extends Aggregate<infer T>
243253
? T
244254
: TSelectObject[K] extends
@@ -366,24 +376,17 @@ export type FunctionalHavingRow<TContext extends Context> = TContext[`schema`] &
366376
(TContext[`result`] extends object ? { $selected: TContext[`result`] } : {})
367377

368378
/**
369-
* RefProxyForContext - Creates ref proxies for all tables/collections in a query context
379+
* RefsForContext - Creates ref proxies for all tables/collections in a query context
370380
*
371381
* This is the main entry point for creating ref objects in query builder callbacks.
372-
* It handles optionality by placing undefined/null OUTSIDE the RefProxy to enable
373-
* JavaScript's optional chaining operator (?.):
382+
* For nullable join sides (left/right/full joins), it produces `Ref<T, true>` instead
383+
* of `Ref<T> | undefined`. This accurately reflects that the proxy object is always
384+
* present at build time (it's a truthy proxy that records property access paths),
385+
* while the `Nullable` flag ensures the result type correctly includes `| undefined`.
374386
*
375387
* Examples:
376-
* - Required field: `RefProxy<User>` → user.name works
377-
* - Optional field: `RefProxy<User> | undefined` → user?.name works
378-
* - Nullable field: `RefProxy<User> | null` → user?.name works
379-
* - Both optional and nullable: `RefProxy<User> | undefined` → user?.name works
380-
*
381-
* The key insight is that `RefProxy<User | undefined>` would NOT allow `user?.name`
382-
* because the undefined is "inside" the proxy, but `RefProxy<User> | undefined`
383-
* does allow it because the undefined is "outside" the proxy.
384-
*
385-
* The logic prioritizes optional chaining by always placing `undefined` outside when
386-
* a type is both optional and nullable (e.g., `string | null | undefined`).
388+
* - Required field: `Ref<User>` → user.name works, result is T
389+
* - Nullable join side: `Ref<User, true>` → user.name works, result is T | undefined
387390
*
388391
* After `select()` is called, this type also includes `$selected` which provides access
389392
* to the SELECT result fields via `$selected.fieldName` syntax.
@@ -394,17 +397,17 @@ export type RefsForContext<TContext extends Context> = {
394397
> extends true
395398
? IsNonExactNullable<TContext[`schema`][K]> extends true
396399
? // T is both non-exact optional and non-exact nullable (e.g., string | null | undefined)
397-
// Extract the non-undefined and non-null part and place undefined outside
398-
Ref<NonNullable<TContext[`schema`][K]>> | undefined
400+
// Extract the non-undefined and non-null part, mark as nullable ref
401+
Ref<NonNullable<TContext[`schema`][K]>, true>
399402
: // T is optional (T | undefined) but not exactly undefined, and not nullable
400-
// Extract the non-undefined part and place undefined outside
401-
Ref<NonUndefined<TContext[`schema`][K]>> | undefined
403+
// Extract the non-undefined part, mark as nullable ref
404+
Ref<NonUndefined<TContext[`schema`][K]>, true>
402405
: IsNonExactNullable<TContext[`schema`][K]> extends true
403406
? // T is nullable (T | null) but not exactly null, and not optional
404-
// Extract the non-null part and place null outside
405-
Ref<NonNull<TContext[`schema`][K]>> | null
407+
// Extract the non-null part, mark as nullable ref
408+
Ref<NonNull<TContext[`schema`][K]>, true>
406409
: // T is exactly undefined, exactly null, or neither optional nor nullable
407-
// Wrap in RefProxy as-is (includes exact undefined, exact null, and normal types)
410+
// Wrap in Ref as-is (includes exact undefined, exact null, and normal types)
408411
Ref<TContext[`schema`][K]>
409412
} & (TContext[`result`] extends object
410413
? { $selected: Ref<TContext[`result`]> }
@@ -479,41 +482,44 @@ type NonNull<T> = T extends null ? never : T
479482
* It provides a recursive interface that allows nested property access while
480483
* preserving optionality and nullability correctly.
481484
*
482-
* When spread in select clauses, it correctly produces the underlying data type
483-
* without Ref wrappers, enabling clean spread operations.
485+
* The `Nullable` parameter indicates whether this ref comes from a nullable
486+
* join side (left/right/full). When `true`, the `Nullable` flag propagates
487+
* through all nested property accesses, ensuring the result type includes
488+
* `| undefined` for all fields accessed through this ref.
484489
*
485490
* Example usage:
486491
* ```typescript
487-
* // Clean interface - no internal properties visible
488-
* const users: Ref<{ id: number; profile?: { bio: string } }> = { ... }
489-
* users.id // Ref<number> - clean display
490-
* users.profile?.bio // Ref<string> - nested optional access works
492+
* // Non-nullable ref (inner join or from table):
493+
* select(({ user }) => ({ name: user.name })) // result: string
494+
*
495+
* // Nullable ref (left join right side):
496+
* select(({ dept }) => ({ name: dept.name })) // result: string | undefined
491497
*
492498
* // Spread operations work cleanly:
493499
* select(({ user }) => ({ ...user })) // Returns User type, not Ref types
494500
* ```
495501
*/
496-
export type Ref<T = any> = {
502+
export type Ref<T = any, Nullable extends boolean = false> = {
497503
[K in keyof T]: IsNonExactOptional<T[K]> extends true
498504
? IsNonExactNullable<T[K]> extends true
499505
? // Both optional and nullable
500506
IsPlainObject<NonNullable<T[K]>> extends true
501-
? Ref<NonNullable<T[K]>> | undefined
502-
: RefLeaf<NonNullable<T[K]>> | undefined
507+
? Ref<NonNullable<T[K]>, Nullable> | undefined
508+
: RefLeaf<NonNullable<T[K]>, Nullable> | undefined
503509
: // Optional only
504510
IsPlainObject<NonUndefined<T[K]>> extends true
505-
? Ref<NonUndefined<T[K]>> | undefined
506-
: RefLeaf<NonUndefined<T[K]>> | undefined
511+
? Ref<NonUndefined<T[K]>, Nullable> | undefined
512+
: RefLeaf<NonUndefined<T[K]>, Nullable> | undefined
507513
: IsNonExactNullable<T[K]> extends true
508514
? // Nullable only
509515
IsPlainObject<NonNull<T[K]>> extends true
510-
? Ref<NonNull<T[K]>> | null
511-
: RefLeaf<NonNull<T[K]>> | null
516+
? Ref<NonNull<T[K]>, Nullable> | null
517+
: RefLeaf<NonNull<T[K]>, Nullable> | null
512518
: // Required
513519
IsPlainObject<T[K]> extends true
514-
? Ref<T[K]>
515-
: RefLeaf<T[K]>
516-
} & RefLeaf<T>
520+
? Ref<T[K], Nullable>
521+
: RefLeaf<T[K], Nullable>
522+
} & RefLeaf<T, Nullable>
517523

518524
/**
519525
* Ref - The user-facing ref type with clean IDE display
@@ -527,11 +533,19 @@ export type Ref<T = any> = {
527533
* - No internal properties like __refProxy, __path, __type are visible
528534
*/
529535
declare const RefBrand: unique symbol
530-
export type RefLeaf<T = any> = { readonly [RefBrand]?: T }
536+
declare const NullableBrand: unique symbol
537+
export type RefLeaf<T = any, Nullable extends boolean = false> = {
538+
readonly [RefBrand]?: T
539+
} & ([Nullable] extends [true] ? { readonly [NullableBrand]?: true } : {})
540+
541+
// Detect NullableBrand by checking for the key's presence
542+
type IsNullableRef<T> = typeof NullableBrand extends keyof T ? true : false
531543

532-
// Helper type to remove RefBrand from objects
544+
// Helper type to remove RefBrand and NullableBrand from objects
533545
type WithoutRefBrand<T> =
534-
T extends Record<string, any> ? Omit<T, typeof RefBrand> : T
546+
T extends Record<string, any>
547+
? Omit<T, typeof RefBrand | typeof NullableBrand>
548+
: T
535549

536550
/**
537551
* PreserveSingleResultFlag - Conditionally includes the singleResult flag

0 commit comments

Comments
 (0)