Skip to content

Commit 7805afb

Browse files
KyleAMathewsclaude
KyleAMathews
and
claude
authored
feat: add runtime validation to prevent custom getKey with joined queries (#717)
* docs: investigate and document custom getKey incompatibility with joins Investigation of bug report where using custom getKey with joined queries causes CollectionOperationError and TransactionError. Root cause: Joined queries use composite keys like "[key1,key2]" internally, but custom getKey returns simple keys, creating a mismatch between the sync system and the collection. Solution: Do not use custom getKey with joined queries. The default getKey correctly uses the composite key from the internal WeakMap. - Added test cases demonstrating correct and incorrect usage - Created comprehensive investigation document with code references - Documented that joined results need composite keys for uniqueness 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * feat: add runtime validation to prevent custom getKey with joined queries Implements fail-fast validation to catch the custom getKey + joins bug at collection creation time instead of during sync. Changes: - Added CustomGetKeyWithJoinError to provide clear error message - Added hasJoins() method that recursively checks query tree for joins - Validation runs in CollectionConfigBuilder constructor - Updated tests to verify error is thrown correctly - Added test for nested subquery join detection The error message guides users to: - Remove custom getKey for joined queries - Use array methods like .toArray.find() instead of .get() Prevents the CollectionOperationError and TransactionError that occurred when sync tried to insert with composite keys "[key1,key2]" while the collection expected simple keys from custom getKey. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore: add changeset and clean up tests for PR - Added changeset for custom getKey validation fix - Removed investigation document (not needed in PR) - Simplified and focused tests on validation behavior - Ran prettier to format code Tests now clearly demonstrate: 1. Joins work without custom getKey 2. Error thrown when custom getKey used with joins 3. Nested subquery joins are detected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: correct test cases for custom getKey validation Fixed TypeScript errors and test logic: - Added .select() to create proper result types for joined queries - Fixed getKey to access the selected properties (baseId instead of id) - Fixed nested subquery test to use actual live query collection instead of function - Properly tests that validation detects joins in nested subqueries All tests now properly validate the runtime error checking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: remove complex nested subquery test case Removed the test for detecting joins in nested live query collections as it requires more complex detection logic that's not implemented yet. The edge case where you reference a live query collection (which internally has joins) would require checking if the source collection is a live query and recursively inspecting its query definition. The two core test cases still validate: 1. Joins work correctly without custom getKey 2. Error is thrown when custom getKey is used with direct joins All tests now pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: improve custom getKey with joins - use contextual errors instead of blocking Implemented Sam's better approach: instead of blocking all custom getKey with joins upfront, we now allow it and provide enhanced error messages when duplicate keys actually occur during sync. Changes: - Removed upfront validation that blocked custom getKey + joins - Enhanced DuplicateKeySyncError with context-aware messaging - Added metadata (_hasCustomGetKey, _hasJoins) to collection utils - Updated sync.ts to pass context when throwing duplicate key errors - Removed unused CustomGetKeyWithJoinError class - Updated tests to show custom getKey works with joins (1:1 cases) - Updated changeset to reflect the new approach Benefits: - Allows valid 1:1 join cases with custom getKey - Only errors when actual duplicates occur (fail-fast on real problems) - Provides helpful guidance with composite key examples - More flexible for users who know their data structure Example enhanced error: "Cannot insert document with key "user1" from sync because it already exists. This collection uses a custom getKey with joined queries. Joined queries can produce multiple rows with the same key when relationships are not 1:1. Consider: (1) using a composite key (e.g., `${item.key1}-${item.key2}`), (2) ensuring your join produces unique rows per key, or (3) removing the custom getKey to use the default composite key behavior." Credit: Suggested by @samwillis 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: clean up changeset to focus on what's being merged Removed references to intermediate solutions and rewrote from the perspective of main branch - what problem this solves and what it adds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: make utils metadata functions to match UtilsRecord type Changed _hasCustomGetKey and _hasJoins from boolean properties to functions that return booleans. This fixes the TypeScript build error since UtilsRecord requires all properties to be functions. - Updated collection-config-builder.ts to use arrow functions - Updated sync.ts to call the functions with () - Tests still pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: address PR review feedback - Replace 'as any' with proper type 'Partial<LiveQueryCollectionUtils>' in sync.ts - Simplify hasJoins to only recurse down 'from' clause, not join subqueries - Remove redundant comment about metadata functions - Add comprehensive test for duplicate key error with custom getKey + joins - Fix ESLint errors with unnecessary optional chaining 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * fix: handle optional user in join test Use optional chaining for u.id in the test to handle the case where the user might be undefined in a join. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * refactor: use Symbol for internal live query utilities Move getBuilder, hasCustomGetKey, and hasJoins behind LIVE_QUERY_INTERNAL Symbol to keep them out of the public API surface. This provides true privacy instead of relying on underscore prefixes. Addresses PR feedback from #717 (comment) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * docs: regenerate API documentation --------- Co-authored-by: Claude <[email protected]>
1 parent 503f0b2 commit 7805afb

73 files changed

Lines changed: 375 additions & 145 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.changeset/prevent-custom-getkey-with-joins.md‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
Improve error messages for custom getKey with joined queries
6+
7+
Enhanced `DuplicateKeySyncError` to provide context-aware guidance when duplicate keys occur with custom `getKey` and joined queries.
8+
9+
**The Issue:**
10+
11+
When using custom `getKey` with joins, duplicate keys can occur if the join produces multiple rows with the same key value. This is valid for 1:1 relationships but problematic for 1:many relationships, and the previous error message didn't explain what went wrong or how to fix it.
12+
13+
**What's New:**
14+
15+
When a duplicate key error occurs in a live query collection that uses both custom `getKey` and joins, the error message now:
16+
17+
- Explains that joined queries can produce multiple rows with the same key
18+
- Suggests using a composite key in your `getKey` function
19+
- Provides concrete examples of solutions
20+
- Helps distinguish between correctly structured 1:1 joins vs problematic 1:many joins
21+
22+
**Example:**
23+
24+
```typescript
25+
// ✅ Valid - 1:1 relationship with unique keys
26+
const userProfiles = createLiveQueryCollection({
27+
query: (q) =>
28+
q
29+
.from({ profile: profiles })
30+
.join({ user: users }, ({ profile, user }) =>
31+
eq(profile.userId, user.id)
32+
),
33+
getKey: (profile) => profile.id, // Each profile has unique ID
34+
})
35+
```
36+
37+
```typescript
38+
// ⚠️ Problematic - 1:many relationship with duplicate keys
39+
const userComments = createLiveQueryCollection({
40+
query: (q) =>
41+
q
42+
.from({ user: users })
43+
.join({ comment: comments }, ({ user, comment }) =>
44+
eq(user.id, comment.userId)
45+
),
46+
getKey: (item) => item.userId, // Multiple comments share same userId!
47+
})
48+
49+
// Enhanced error message:
50+
// "Cannot insert document with key "user1" from sync because it already exists.
51+
// This collection uses a custom getKey with joined queries. Joined queries can
52+
// produce multiple rows with the same key when relationships are not 1:1.
53+
// Consider: (1) using a composite key in your getKey function (e.g., `${item.key1}-${item.key2}`),
54+
// (2) ensuring your join produces unique rows per key, or (3) removing the
55+
// custom getKey to use the default composite key behavior."
56+
```

‎docs/reference/classes/AggregateFunctionNotInSelectError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: AggregateFunctionNotInSelectError
55

66
# Class: AggregateFunctionNotInSelectError
77

8-
Defined in: [packages/db/src/errors.ts:531](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L531)
8+
Defined in: [packages/db/src/errors.ts:547](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L547)
99

1010
## Extends
1111

@@ -19,7 +19,7 @@ Defined in: [packages/db/src/errors.ts:531](https://github.com/TanStack/db/blob/
1919
new AggregateFunctionNotInSelectError(functionName): AggregateFunctionNotInSelectError;
2020
```
2121

22-
Defined in: [packages/db/src/errors.ts:532](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L532)
22+
Defined in: [packages/db/src/errors.ts:548](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L548)
2323

2424
#### Parameters
2525

‎docs/reference/classes/AggregateNotSupportedError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: AggregateNotSupportedError
55

66
# Class: AggregateNotSupportedError
77

8-
Defined in: [packages/db/src/errors.ts:647](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L647)
8+
Defined in: [packages/db/src/errors.ts:663](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L663)
99

1010
Error thrown when aggregate expressions are used outside of a GROUP BY context.
1111

@@ -21,7 +21,7 @@ Error thrown when aggregate expressions are used outside of a GROUP BY context.
2121
new AggregateNotSupportedError(): AggregateNotSupportedError;
2222
```
2323

24-
Defined in: [packages/db/src/errors.ts:648](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L648)
24+
Defined in: [packages/db/src/errors.ts:664](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L664)
2525

2626
#### Returns
2727

‎docs/reference/classes/CannotCombineEmptyExpressionListError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: CannotCombineEmptyExpressionListError
55

66
# Class: CannotCombineEmptyExpressionListError
77

8-
Defined in: [packages/db/src/errors.ts:610](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L610)
8+
Defined in: [packages/db/src/errors.ts:626](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L626)
99

1010
## Extends
1111

@@ -19,7 +19,7 @@ Defined in: [packages/db/src/errors.ts:610](https://github.com/TanStack/db/blob/
1919
new CannotCombineEmptyExpressionListError(): CannotCombineEmptyExpressionListError;
2020
```
2121

22-
Defined in: [packages/db/src/errors.ts:611](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L611)
22+
Defined in: [packages/db/src/errors.ts:627](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L627)
2323

2424
#### Returns
2525

‎docs/reference/classes/CollectionInputNotFoundError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: CollectionInputNotFoundError
55

66
# Class: CollectionInputNotFoundError
77

8-
Defined in: [packages/db/src/errors.ts:391](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L391)
8+
Defined in: [packages/db/src/errors.ts:407](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L407)
99

1010
Error thrown when a collection input stream is not found during query compilation.
1111
In self-joins, each alias (e.g., 'employee', 'manager') requires its own input stream.
@@ -25,7 +25,7 @@ new CollectionInputNotFoundError(
2525
availableKeys?): CollectionInputNotFoundError;
2626
```
2727
28-
Defined in: [packages/db/src/errors.ts:392](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L392)
28+
Defined in: [packages/db/src/errors.ts:408](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L408)
2929
3030
#### Parameters
3131

‎docs/reference/classes/DeleteKeyNotFoundError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: DeleteKeyNotFoundError
55

66
# Class: DeleteKeyNotFoundError
77

8-
Defined in: [packages/db/src/errors.ts:204](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L204)
8+
Defined in: [packages/db/src/errors.ts:220](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L220)
99

1010
## Extends
1111

@@ -19,7 +19,7 @@ Defined in: [packages/db/src/errors.ts:204](https://github.com/TanStack/db/blob/
1919
new DeleteKeyNotFoundError(key): DeleteKeyNotFoundError;
2020
```
2121

22-
Defined in: [packages/db/src/errors.ts:205](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L205)
22+
Defined in: [packages/db/src/errors.ts:221](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L221)
2323

2424
#### Parameters
2525

‎docs/reference/classes/DistinctRequiresSelectError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: DistinctRequiresSelectError
55

66
# Class: DistinctRequiresSelectError
77

8-
Defined in: [packages/db/src/errors.ts:367](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L367)
8+
Defined in: [packages/db/src/errors.ts:383](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L383)
99

1010
## Extends
1111

@@ -19,7 +19,7 @@ Defined in: [packages/db/src/errors.ts:367](https://github.com/TanStack/db/blob/
1919
new DistinctRequiresSelectError(): DistinctRequiresSelectError;
2020
```
2121

22-
Defined in: [packages/db/src/errors.ts:368](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L368)
22+
Defined in: [packages/db/src/errors.ts:384](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L384)
2323

2424
#### Returns
2525

‎docs/reference/classes/DuplicateAliasInSubqueryError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: DuplicateAliasInSubqueryError
55

66
# Class: DuplicateAliasInSubqueryError
77

8-
Defined in: [packages/db/src/errors.ts:412](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L412)
8+
Defined in: [packages/db/src/errors.ts:428](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L428)
99

1010
Error thrown when a subquery uses the same alias as its parent query.
1111
This causes issues because parent and subquery would share the same input streams,
@@ -23,7 +23,7 @@ leading to empty results or incorrect data (aggregation cross-leaking).
2323
new DuplicateAliasInSubqueryError(alias, parentAliases): DuplicateAliasInSubqueryError;
2424
```
2525

26-
Defined in: [packages/db/src/errors.ts:413](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L413)
26+
Defined in: [packages/db/src/errors.ts:429](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L429)
2727

2828
#### Parameters
2929

‎docs/reference/classes/DuplicateKeySyncError.md‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ Defined in: [packages/db/src/errors.ts:162](https://github.com/TanStack/db/blob/
1616
### Constructor
1717

1818
```ts
19-
new DuplicateKeySyncError(key, collectionId): DuplicateKeySyncError;
19+
new DuplicateKeySyncError(
20+
key,
21+
collectionId,
22+
options?): DuplicateKeySyncError;
2023
```
2124
2225
Defined in: [packages/db/src/errors.ts:163](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L163)
@@ -31,6 +34,16 @@ Defined in: [packages/db/src/errors.ts:163](https://github.com/TanStack/db/blob/
3134
3235
`string`
3336
37+
##### options?
38+
39+
###### hasCustomGetKey?
40+
41+
`boolean`
42+
43+
###### hasJoins?
44+
45+
`boolean`
46+
3447
#### Returns
3548
3649
`DuplicateKeySyncError`

‎docs/reference/classes/EmptyReferencePathError.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ title: EmptyReferencePathError
55

66
# Class: EmptyReferencePathError
77

8-
Defined in: [packages/db/src/errors.ts:435](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L435)
8+
Defined in: [packages/db/src/errors.ts:451](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L451)
99

1010
## Extends
1111

@@ -19,7 +19,7 @@ Defined in: [packages/db/src/errors.ts:435](https://github.com/TanStack/db/blob/
1919
new EmptyReferencePathError(): EmptyReferencePathError;
2020
```
2121

22-
Defined in: [packages/db/src/errors.ts:436](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L436)
22+
Defined in: [packages/db/src/errors.ts:452](https://github.com/TanStack/db/blob/main/packages/db/src/errors.ts#L452)
2323

2424
#### Returns
2525

0 commit comments

Comments
 (0)