Skip to content

Commit 464805d

Browse files
KyleAMathewsclaude
andauthored
Throw a clear error when anything other than an object with an collection is passed to "from" (#875)
* Improve error message when invalid source type is passed to .from() Fixes #873 The issue was that users were getting a confusing error message when passing a string (or other invalid type) to the .from() method instead of an object with a collection. For example: ```javascript // Incorrect usage q.from("conversations") // String instead of object // Correct usage q.from({ conversations: conversationsCollection }) ``` When a string was passed, Object.keys("conversations") would return an array of character indices ['0', '1', ...], which has length > 1, triggering the "Only one source is allowed in the from clause" error. This was misleading because the real issue was passing a string instead of an object. Changes: - Added validation to check if the source is a plain object before checking its key count - Improved error message to explicitly state the expected format with an example: .from({ alias: collection }) - Added comprehensive tests for various invalid input types (string, null, array, undefined) The new error message is: "Invalid source for from clause: Expected an object with a single key-value pair like { alias: collection }. For example: .from({ todos: todosCollection }). Got: string \"conversations\"" * Refactor error validation to use positive checks Simplified the validation logic in _createRefForSource to use positive checks instead of negative checks: - Check if it's a valid object (handles null/undefined via try-catch) - Check if it's an array (arrays pass Object.keys but aren't valid) - Check if it has exactly one key - Check if keys look like numeric indices (indicates string was passed) This approach is cleaner and handles all edge cases including when callers bypass TypeScript's type checks with 'as any'. Also added a changeset documenting the improvement. * Move error messages to dedicated error class Addresses code review feedback by creating InvalidSourceTypeError class in errors.ts and using it consistently across all validation cases instead of duplicating error message strings. Changes: - Added InvalidSourceTypeError class to errors.ts that takes context and type parameters - Updated _createRefForSource to use InvalidSourceTypeError in all four validation cases (null/undefined, array, empty object, string) - Updated tests to expect InvalidSourceTypeError instead of QueryBuilderError - This eliminates string duplication and makes error messages consistent across .from() and .join() methods --------- Co-authored-by: Claude <[email protected]>
1 parent f15b2a7 commit 464805d

File tree

4 files changed

+99
-2
lines changed

4 files changed

+99
-2
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
Improved error messages when invalid source types are passed to `.from()` or `.join()` methods. When users mistakenly pass a string, null, array, or other invalid type instead of an object with a collection, they now receive a clear, actionable error message with an example of the correct usage (e.g., `.from({ todos: todosCollection })`).

packages/db/src/errors.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,15 @@ export class InvalidSourceError extends QueryBuilderError {
360360
}
361361
}
362362

363+
export class InvalidSourceTypeError extends QueryBuilderError {
364+
constructor(context: string, type: string) {
365+
super(
366+
`Invalid source for ${context}: Expected an object with a single key-value pair like { alias: collection }. ` +
367+
`For example: .from({ todos: todosCollection }). Got: ${type}`
368+
)
369+
}
370+
}
371+
363372
export class JoinConditionMustBeEqualityError extends QueryBuilderError {
364373
constructor() {
365374
super(`Join condition must be an equality expression`)

packages/db/src/query/builder/index.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from "../ir.js"
1111
import {
1212
InvalidSourceError,
13+
InvalidSourceTypeError,
1314
JoinConditionMustBeEqualityError,
1415
OnlyOneSourceAllowedError,
1516
QueryMustHaveFromClauseError,
@@ -60,13 +61,38 @@ export class BaseQueryBuilder<TContext extends Context = Context> {
6061
source: TSource,
6162
context: string
6263
): [string, CollectionRef | QueryRef] {
63-
if (Object.keys(source).length !== 1) {
64+
// Validate source is a plain object (not null, array, string, etc.)
65+
// We use try-catch to handle null/undefined gracefully
66+
let keys: Array<string>
67+
try {
68+
keys = Object.keys(source)
69+
} catch {
70+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
71+
const type = source === null ? `null` : `undefined`
72+
throw new InvalidSourceTypeError(context, type)
73+
}
74+
75+
// Check if it's an array (arrays pass Object.keys but aren't valid sources)
76+
if (Array.isArray(source)) {
77+
throw new InvalidSourceTypeError(context, `array`)
78+
}
79+
80+
// Validate exactly one key
81+
if (keys.length !== 1) {
82+
if (keys.length === 0) {
83+
throw new InvalidSourceTypeError(context, `empty object`)
84+
}
85+
// Check if it looks like a string was passed (has numeric keys)
86+
if (keys.every((k) => !isNaN(Number(k)))) {
87+
throw new InvalidSourceTypeError(context, `string`)
88+
}
6489
throw new OnlyOneSourceAllowedError(context)
6590
}
6691

67-
const alias = Object.keys(source)[0]!
92+
const alias = keys[0]!
6893
const sourceValue = source[alias]
6994

95+
// Validate the value is a Collection or QueryBuilder
7096
let ref: CollectionRef | QueryRef
7197

7298
if (sourceValue instanceof CollectionImpl) {

packages/db/tests/query/builder/from.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { CollectionImpl } from "../../../src/collection/index.js"
33
import { Query, getQueryIR } from "../../../src/query/builder/index.js"
44
import { eq } from "../../../src/query/builder/functions.js"
55
import {
6+
InvalidSourceTypeError,
67
OnlyOneSourceAllowedError,
78
QueryMustHaveFromClauseError,
89
} from "../../../src/errors"
@@ -108,4 +109,60 @@ describe(`QueryBuilder.from`, () => {
108109
} as any)
109110
}).toThrow(OnlyOneSourceAllowedError)
110111
})
112+
113+
it(`throws helpful error when passing a string instead of an object`, () => {
114+
const builder = new Query()
115+
116+
expect(() => {
117+
builder.from(`employees` as any)
118+
}).toThrow(InvalidSourceTypeError)
119+
120+
expect(() => {
121+
builder.from(`employees` as any)
122+
}).toThrow(
123+
/Invalid source for from clause: Expected an object with a single key-value pair/
124+
)
125+
})
126+
127+
it(`throws helpful error when passing null`, () => {
128+
const builder = new Query()
129+
130+
expect(() => {
131+
builder.from(null as any)
132+
}).toThrow(InvalidSourceTypeError)
133+
134+
expect(() => {
135+
builder.from(null as any)
136+
}).toThrow(
137+
/Invalid source for from clause: Expected an object with a single key-value pair/
138+
)
139+
})
140+
141+
it(`throws helpful error when passing an array`, () => {
142+
const builder = new Query()
143+
144+
expect(() => {
145+
builder.from([employeesCollection] as any)
146+
}).toThrow(InvalidSourceTypeError)
147+
148+
expect(() => {
149+
builder.from([employeesCollection] as any)
150+
}).toThrow(
151+
/Invalid source for from clause: Expected an object with a single key-value pair/
152+
)
153+
})
154+
155+
it(`throws helpful error when passing undefined`, () => {
156+
const builder = new Query()
157+
158+
expect(() => {
159+
builder.from(undefined as any)
160+
}).toThrow(InvalidSourceTypeError)
161+
162+
expect(() => {
163+
builder.from(undefined as any)
164+
}).toThrow(
165+
/Invalid source for from clause: Expected an object with a single key-value pair/
166+
)
167+
})
111168
})

0 commit comments

Comments
 (0)