From 1b026ac286771d77ca54be6813888de987ac08dc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 15 Dec 2025 14:03:50 +0000 Subject: [PATCH 1/4] feat(db): throw error when JavaScript operators used in queries Adds detection and helpful error messages when users mistakenly use JavaScript operators (||, &&, ??) in query callbacks. These operators are evaluated at query construction time, not execution time, causing silent unexpected behavior. Changes: - Add JavaScriptOperatorInQueryError with helpful suggestions - Add Symbol.toPrimitive trap to RefProxy for primitive coercion - Add checkCallbackForJsOperators() to detect operators in callbacks - Integrate checks into select(), where(), and having() methods - Add comprehensive tests for the new error detection - Fix existing tests that incorrectly used JS operators --- packages/db/src/errors.ts | 24 ++++ packages/db/src/query/builder/index.ts | 11 +- packages/db/src/query/builder/ref-proxy.ts | 110 ++++++++++++++++++ .../db/tests/query/builder/ref-proxy.test.ts | 79 +++++++++++++ .../tests/query/live-query-collection.test.ts | 4 +- packages/db/tests/query/scheduler.test.ts | 4 +- 6 files changed, 227 insertions(+), 5 deletions(-) diff --git a/packages/db/src/errors.ts b/packages/db/src/errors.ts index 50cd442ac..592307502 100644 --- a/packages/db/src/errors.ts +++ b/packages/db/src/errors.ts @@ -398,6 +398,30 @@ export class QueryCompilationError extends TanStackDBError { } } +export class JavaScriptOperatorInQueryError extends QueryBuilderError { + constructor(operator: string, hint?: string) { + const defaultHint = + operator === `||` || operator === `??` + ? `Use coalesce() instead: coalesce(value, defaultValue)` + : operator === `&&` + ? `Use and() for logical conditions` + : operator === `?:` + ? `Use cond() for conditional expressions: cond(condition, trueValue, falseValue)` + : `Use the appropriate query function instead` + + super( + `JavaScript operator "${operator}" cannot be used in queries.\n\n` + + `Query callbacks should only use field references and query functions, not JavaScript logic.\n` + + `${hint || defaultHint}\n\n` + + `Example of incorrect usage:\n` + + ` .select(({users}) => ({ data: users.data || [] }))\n\n` + + `Correct usage:\n` + + ` .select(({users}) => ({ data: coalesce(users.data, []) }))`, + ) + this.name = `JavaScriptOperatorInQueryError` + } +} + export class DistinctRequiresSelectError extends QueryCompilationError { constructor() { super(`DISTINCT requires a SELECT clause.`) diff --git a/packages/db/src/query/builder/index.ts b/packages/db/src/query/builder/index.ts index e7d2be0c4..eed5af0aa 100644 --- a/packages/db/src/query/builder/index.ts +++ b/packages/db/src/query/builder/index.ts @@ -16,7 +16,7 @@ import { QueryMustHaveFromClauseError, SubQueryMustHaveFromClauseError, } from '../../errors.js' -import { createRefProxy, toExpression } from './ref-proxy.js' +import { checkCallbackForJsOperators, createRefProxy, toExpression } from './ref-proxy.js' import type { NamespacedRow, SingleResult } from '../../types.js' import type { Aggregate, @@ -357,6 +357,9 @@ export class BaseQueryBuilder { * ``` */ where(callback: WhereCallback): QueryBuilder { + // Check for JavaScript operators that cannot be translated to query operations + checkCallbackForJsOperators(callback) + const aliases = this._getCurrentAliases() const refProxy = createRefProxy(aliases) as RefsForContext const expression = callback(refProxy) @@ -398,6 +401,9 @@ export class BaseQueryBuilder { * ``` */ having(callback: WhereCallback): QueryBuilder { + // Check for JavaScript operators that cannot be translated to query operations + checkCallbackForJsOperators(callback) + const aliases = this._getCurrentAliases() const refProxy = createRefProxy(aliases) as RefsForContext const expression = callback(refProxy) @@ -447,6 +453,9 @@ export class BaseQueryBuilder { select( callback: (refs: RefsForContext) => TSelectObject, ): QueryBuilder>> { + // Check for JavaScript operators that cannot be translated to query operations + checkCallbackForJsOperators(callback) + const aliases = this._getCurrentAliases() const refProxy = createRefProxy(aliases) as RefsForContext const selectObject = callback(refProxy) diff --git a/packages/db/src/query/builder/ref-proxy.ts b/packages/db/src/query/builder/ref-proxy.ts index 2f060b5f2..5a0d34161 100644 --- a/packages/db/src/query/builder/ref-proxy.ts +++ b/packages/db/src/query/builder/ref-proxy.ts @@ -1,7 +1,24 @@ import { PropRef, Value } from '../ir.js' +import { JavaScriptOperatorInQueryError } from '../../errors.js' import type { BasicExpression } from '../ir.js' import type { RefLeaf } from './types.js' +/** + * Creates a handler for Symbol.toPrimitive that throws an error when + * JavaScript tries to coerce a RefProxy to a primitive value. + * This catches misuse like string concatenation, arithmetic, etc. + */ +function createToPrimitiveHandler(path: Array): (hint: string) => never { + return (hint: string) => { + const pathStr = path.length > 0 ? path.join(`.`) : `` + throw new JavaScriptOperatorInQueryError( + hint === `number` ? `arithmetic` : hint === `string` ? `string concatenation` : `comparison`, + `Attempted to use "${pathStr}" in a JavaScript ${hint} context.\n` + + `Query references can only be used with query functions, not JavaScript operators.`, + ) + } +} + export interface RefProxy { /** @internal */ readonly __refProxy: true @@ -44,6 +61,10 @@ export function createSingleRowRefProxy< if (prop === `__refProxy`) return true if (prop === `__path`) return path if (prop === `__type`) return undefined // Type is only for TypeScript inference + // Intercept Symbol.toPrimitive to catch JS coercion attempts + if (prop === Symbol.toPrimitive) { + return createToPrimitiveHandler(path) + } if (typeof prop === `symbol`) return Reflect.get(target, prop, receiver) const newPath = [...path, String(prop)] @@ -97,6 +118,10 @@ export function createRefProxy>( if (prop === `__refProxy`) return true if (prop === `__path`) return path if (prop === `__type`) return undefined // Type is only for TypeScript inference + // Intercept Symbol.toPrimitive to catch JS coercion attempts + if (prop === Symbol.toPrimitive) { + return createToPrimitiveHandler(path) + } if (typeof prop === `symbol`) return Reflect.get(target, prop, receiver) const newPath = [...path, String(prop)] @@ -140,6 +165,10 @@ export function createRefProxy>( if (prop === `__refProxy`) return true if (prop === `__path`) return [] if (prop === `__type`) return undefined // Type is only for TypeScript inference + // Intercept Symbol.toPrimitive to catch JS coercion attempts + if (prop === Symbol.toPrimitive) { + return createToPrimitiveHandler([]) + } if (typeof prop === `symbol`) return Reflect.get(target, prop, receiver) const propStr = String(prop) @@ -213,3 +242,84 @@ export function isRefProxy(value: any): value is RefProxy { export function val(value: T): BasicExpression { return new Value(value) } + +/** + * Patterns that indicate JavaScript operators being used in query callbacks. + * These operators cannot be translated to query operations and will silently + * produce incorrect results. + */ +const JS_OPERATOR_PATTERNS: Array<{ + pattern: RegExp + operator: string + description: string +}> = [ + { + // Match || that's not inside a string or comment + // This regex looks for || not preceded by quotes that would indicate a string + pattern: /\|\|/, + operator: `||`, + description: `logical OR`, + }, + { + // Match && that's not inside a string or comment + pattern: /&&/, + operator: `&&`, + description: `logical AND`, + }, + { + // Match ?? nullish coalescing + pattern: /\?\?/, + operator: `??`, + description: `nullish coalescing`, + }, +] + +/** + * Removes string literals and comments from source code to avoid false positives + * when checking for JavaScript operators. + */ +function stripStringsAndComments(source: string): string { + // Remove template literals (backtick strings) + let result = source.replace(/`(?:[^`\\]|\\.)*`/g, `""`) + // Remove double-quoted strings + result = result.replace(/"(?:[^"\\]|\\.)*"/g, `""`) + // Remove single-quoted strings + result = result.replace(/'(?:[^'\\]|\\.)*'/g, `""`) + // Remove single-line comments + result = result.replace(/\/\/[^\n]*/g, ``) + // Remove multi-line comments + result = result.replace(/\/\*[\s\S]*?\*\//g, ``) + return result +} + +/** + * Checks a callback function's source code for JavaScript operators that + * cannot be translated to query operations. + * + * @param callback - The callback function to check + * @throws JavaScriptOperatorInQueryError if a problematic operator is found + * + * @example + * // This will throw an error: + * checkCallbackForJsOperators(({users}) => users.data || []) + * + * // This is fine: + * checkCallbackForJsOperators(({users}) => users.data) + */ +export function checkCallbackForJsOperators(callback: (...args: Array) => unknown): void { + const source = callback.toString() + + // Strip strings and comments to avoid false positives + const cleanedSource = stripStringsAndComments(source) + + for (const { pattern, operator, description } of JS_OPERATOR_PATTERNS) { + if (pattern.test(cleanedSource)) { + throw new JavaScriptOperatorInQueryError( + operator, + `Found JavaScript ${description} operator (${operator}) in query callback.\n` + + `This operator is evaluated at query construction time, not at query execution time,\n` + + `which means it will not behave as expected.`, + ) + } + } +} diff --git a/packages/db/tests/query/builder/ref-proxy.test.ts b/packages/db/tests/query/builder/ref-proxy.test.ts index ec41351da..0c15016c1 100644 --- a/packages/db/tests/query/builder/ref-proxy.test.ts +++ b/packages/db/tests/query/builder/ref-proxy.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it } from 'vitest' import { + checkCallbackForJsOperators, createRefProxy, isRefProxy, toExpression, val, } from '../../../src/query/builder/ref-proxy.js' import { PropRef, Value } from '../../../src/query/ir.js' +import { JavaScriptOperatorInQueryError } from '../../../src/errors.js' describe(`ref-proxy`, () => { describe(`createRefProxy`, () => { @@ -214,4 +216,81 @@ describe(`ref-proxy`, () => { expect((val({ a: 1 }) as Value).value).toEqual({ a: 1 }) }) }) + + describe(`checkCallbackForJsOperators`, () => { + it(`throws error for || operator`, () => { + const callback = ({ users }: any) => ({ data: users.data || [] }) + expect(() => checkCallbackForJsOperators(callback)).toThrow( + JavaScriptOperatorInQueryError, + ) + expect(() => checkCallbackForJsOperators(callback)).toThrow(`||`) + }) + + it(`throws error for && operator`, () => { + const callback = ({ users }: any) => users.active && users.name + expect(() => checkCallbackForJsOperators(callback)).toThrow( + JavaScriptOperatorInQueryError, + ) + expect(() => checkCallbackForJsOperators(callback)).toThrow(`&&`) + }) + + it(`throws error for ?? operator`, () => { + const callback = ({ users }: any) => ({ name: users.name ?? `default` }) + expect(() => checkCallbackForJsOperators(callback)).toThrow( + JavaScriptOperatorInQueryError, + ) + expect(() => checkCallbackForJsOperators(callback)).toThrow(`??`) + }) + + it(`does not throw for valid query callbacks`, () => { + // Simple property access + expect(() => + checkCallbackForJsOperators(({ users }: any) => users.name), + ).not.toThrow() + + // Object with property access + expect(() => + checkCallbackForJsOperators(({ users }: any) => ({ + id: users.id, + name: users.name, + })), + ).not.toThrow() + + // Optional chaining is allowed + expect(() => + checkCallbackForJsOperators(({ users }: any) => users.profile?.bio), + ).not.toThrow() + }) + + it(`does not throw for operators in string literals`, () => { + // || in a string literal should not trigger error + expect(() => + checkCallbackForJsOperators(() => ({ message: `a || b is valid` })), + ).not.toThrow() + + // && in a string literal should not trigger error + expect(() => + checkCallbackForJsOperators(() => ({ message: `a && b is valid` })), + ).not.toThrow() + }) + }) + + describe(`Symbol.toPrimitive trap`, () => { + it(`throws error when proxy is coerced to string`, () => { + const proxy = createRefProxy<{ users: { id: number } }>([`users`]) + expect(() => String(proxy.users.id)).toThrow(JavaScriptOperatorInQueryError) + }) + + it(`throws error when proxy is used in arithmetic`, () => { + const proxy = createRefProxy<{ users: { id: number } }>([`users`]) + expect(() => Number(proxy.users.id)).toThrow(JavaScriptOperatorInQueryError) + }) + + it(`throws error when proxy is concatenated with string`, () => { + const proxy = createRefProxy<{ users: { name: string } }>([`users`]) + expect(() => `Hello ${proxy.users.name}`).toThrow( + JavaScriptOperatorInQueryError, + ) + }) + }) }) diff --git a/packages/db/tests/query/live-query-collection.test.ts b/packages/db/tests/query/live-query-collection.test.ts index 8c10c3eb8..df286760b 100644 --- a/packages/db/tests/query/live-query-collection.test.ts +++ b/packages/db/tests/query/live-query-collection.test.ts @@ -1911,9 +1911,9 @@ describe(`createLiveQueryCollection`, () => { .join({ users }, ({ comments: c, users: u }) => eq(c.userId, u.id)) .select(({ comments: c, users: u }) => ({ id: c.id, - userId: u?.id ?? c.userId, + userId: u.id, text: c.text, - userName: u?.name, + userName: u.name, })), getKey: (item) => item.userId, startSync: true, diff --git a/packages/db/tests/query/scheduler.test.ts b/packages/db/tests/query/scheduler.test.ts index adb85c979..2de85931e 100644 --- a/packages/db/tests/query/scheduler.test.ts +++ b/packages/db/tests/query/scheduler.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { createCollection } from '../../src/collection/index.js' -import { createLiveQueryCollection, eq, isNull } from '../../src/query/index.js' +import { coalesce, createLiveQueryCollection, eq, isNull } from '../../src/query/index.js' import { createTransaction } from '../../src/transactions.js' import { createOptimisticAction } from '../../src/optimistic-action.js' import { transactionScopedScheduler } from '../../src/scheduler.js' @@ -721,7 +721,7 @@ describe(`live query scheduler`, () => { .select(({ a, b }) => ({ id: a.id, aValue: a.value, - bValue: b?.value ?? null, + bValue: coalesce(b!.value, null), })), }) From ddc3b47321207aafe64a2e6cb676637aa8a831c3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 14:08:35 +0000 Subject: [PATCH 2/4] ci: apply automated fixes --- packages/db/src/query/builder/index.ts | 6 +++++- packages/db/src/query/builder/ref-proxy.ts | 14 +++++++++++--- packages/db/tests/query/builder/ref-proxy.test.ts | 8 ++++++-- packages/db/tests/query/scheduler.test.ts | 7 ++++++- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/db/src/query/builder/index.ts b/packages/db/src/query/builder/index.ts index eed5af0aa..033beac51 100644 --- a/packages/db/src/query/builder/index.ts +++ b/packages/db/src/query/builder/index.ts @@ -16,7 +16,11 @@ import { QueryMustHaveFromClauseError, SubQueryMustHaveFromClauseError, } from '../../errors.js' -import { checkCallbackForJsOperators, createRefProxy, toExpression } from './ref-proxy.js' +import { + checkCallbackForJsOperators, + createRefProxy, + toExpression, +} from './ref-proxy.js' import type { NamespacedRow, SingleResult } from '../../types.js' import type { Aggregate, diff --git a/packages/db/src/query/builder/ref-proxy.ts b/packages/db/src/query/builder/ref-proxy.ts index 5a0d34161..31b44b9aa 100644 --- a/packages/db/src/query/builder/ref-proxy.ts +++ b/packages/db/src/query/builder/ref-proxy.ts @@ -8,11 +8,17 @@ import type { RefLeaf } from './types.js' * JavaScript tries to coerce a RefProxy to a primitive value. * This catches misuse like string concatenation, arithmetic, etc. */ -function createToPrimitiveHandler(path: Array): (hint: string) => never { +function createToPrimitiveHandler( + path: Array, +): (hint: string) => never { return (hint: string) => { const pathStr = path.length > 0 ? path.join(`.`) : `` throw new JavaScriptOperatorInQueryError( - hint === `number` ? `arithmetic` : hint === `string` ? `string concatenation` : `comparison`, + hint === `number` + ? `arithmetic` + : hint === `string` + ? `string concatenation` + : `comparison`, `Attempted to use "${pathStr}" in a JavaScript ${hint} context.\n` + `Query references can only be used with query functions, not JavaScript operators.`, ) @@ -306,7 +312,9 @@ function stripStringsAndComments(source: string): string { * // This is fine: * checkCallbackForJsOperators(({users}) => users.data) */ -export function checkCallbackForJsOperators(callback: (...args: Array) => unknown): void { +export function checkCallbackForJsOperators( + callback: (...args: Array) => unknown, +): void { const source = callback.toString() // Strip strings and comments to avoid false positives diff --git a/packages/db/tests/query/builder/ref-proxy.test.ts b/packages/db/tests/query/builder/ref-proxy.test.ts index 0c15016c1..050472125 100644 --- a/packages/db/tests/query/builder/ref-proxy.test.ts +++ b/packages/db/tests/query/builder/ref-proxy.test.ts @@ -278,12 +278,16 @@ describe(`ref-proxy`, () => { describe(`Symbol.toPrimitive trap`, () => { it(`throws error when proxy is coerced to string`, () => { const proxy = createRefProxy<{ users: { id: number } }>([`users`]) - expect(() => String(proxy.users.id)).toThrow(JavaScriptOperatorInQueryError) + expect(() => String(proxy.users.id)).toThrow( + JavaScriptOperatorInQueryError, + ) }) it(`throws error when proxy is used in arithmetic`, () => { const proxy = createRefProxy<{ users: { id: number } }>([`users`]) - expect(() => Number(proxy.users.id)).toThrow(JavaScriptOperatorInQueryError) + expect(() => Number(proxy.users.id)).toThrow( + JavaScriptOperatorInQueryError, + ) }) it(`throws error when proxy is concatenated with string`, () => { diff --git a/packages/db/tests/query/scheduler.test.ts b/packages/db/tests/query/scheduler.test.ts index 2de85931e..d53798486 100644 --- a/packages/db/tests/query/scheduler.test.ts +++ b/packages/db/tests/query/scheduler.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { createCollection } from '../../src/collection/index.js' -import { coalesce, createLiveQueryCollection, eq, isNull } from '../../src/query/index.js' +import { + coalesce, + createLiveQueryCollection, + eq, + isNull, +} from '../../src/query/index.js' import { createTransaction } from '../../src/transactions.js' import { createOptimisticAction } from '../../src/optimistic-action.js' import { transactionScopedScheduler } from '../../src/scheduler.js' From 05ea0100e19a365d678c008d12c418384dc7077b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 15 Dec 2025 14:19:52 +0000 Subject: [PATCH 3/4] fix(db): fix TypeScript type error in checkCallbackForJsOperators --- packages/db/src/query/builder/ref-proxy.ts | 4 ++-- packages/db/tests/query/live-query-collection.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/db/src/query/builder/ref-proxy.ts b/packages/db/src/query/builder/ref-proxy.ts index 31b44b9aa..498cdd2f1 100644 --- a/packages/db/src/query/builder/ref-proxy.ts +++ b/packages/db/src/query/builder/ref-proxy.ts @@ -312,8 +312,8 @@ function stripStringsAndComments(source: string): string { * // This is fine: * checkCallbackForJsOperators(({users}) => users.data) */ -export function checkCallbackForJsOperators( - callback: (...args: Array) => unknown, +export function checkCallbackForJsOperators) => any>( + callback: T, ): void { const source = callback.toString() diff --git a/packages/db/tests/query/live-query-collection.test.ts b/packages/db/tests/query/live-query-collection.test.ts index df286760b..bc2c8bbeb 100644 --- a/packages/db/tests/query/live-query-collection.test.ts +++ b/packages/db/tests/query/live-query-collection.test.ts @@ -1911,9 +1911,9 @@ describe(`createLiveQueryCollection`, () => { .join({ users }, ({ comments: c, users: u }) => eq(c.userId, u.id)) .select(({ comments: c, users: u }) => ({ id: c.id, - userId: u.id, + userId: u!.id, text: c.text, - userName: u.name, + userName: u!.name, })), getKey: (item) => item.userId, startSync: true, From 7c6c5377192fd99f94afd5a342b9f24409ccf2a5 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 14:21:48 +0000 Subject: [PATCH 4/4] ci: apply automated fixes --- packages/db/src/query/builder/ref-proxy.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/db/src/query/builder/ref-proxy.ts b/packages/db/src/query/builder/ref-proxy.ts index 498cdd2f1..318de0943 100644 --- a/packages/db/src/query/builder/ref-proxy.ts +++ b/packages/db/src/query/builder/ref-proxy.ts @@ -312,9 +312,9 @@ function stripStringsAndComments(source: string): string { * // This is fine: * checkCallbackForJsOperators(({users}) => users.data) */ -export function checkCallbackForJsOperators) => any>( - callback: T, -): void { +export function checkCallbackForJsOperators< + T extends (...args: Array) => any, +>(callback: T): void { const source = callback.toString() // Strip strings and comments to avoid false positives