Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions packages/db/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`)
Expand Down
15 changes: 14 additions & 1 deletion packages/db/src/query/builder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ 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,
Expand Down Expand Up @@ -357,6 +361,9 @@ export class BaseQueryBuilder<TContext extends Context = Context> {
* ```
*/
where(callback: WhereCallback<TContext>): QueryBuilder<TContext> {
// Check for JavaScript operators that cannot be translated to query operations
checkCallbackForJsOperators(callback)

const aliases = this._getCurrentAliases()
const refProxy = createRefProxy(aliases) as RefsForContext<TContext>
const expression = callback(refProxy)
Expand Down Expand Up @@ -398,6 +405,9 @@ export class BaseQueryBuilder<TContext extends Context = Context> {
* ```
*/
having(callback: WhereCallback<TContext>): QueryBuilder<TContext> {
// Check for JavaScript operators that cannot be translated to query operations
checkCallbackForJsOperators(callback)

const aliases = this._getCurrentAliases()
const refProxy = createRefProxy(aliases) as RefsForContext<TContext>
const expression = callback(refProxy)
Expand Down Expand Up @@ -447,6 +457,9 @@ export class BaseQueryBuilder<TContext extends Context = Context> {
select<TSelectObject extends SelectObject>(
callback: (refs: RefsForContext<TContext>) => TSelectObject,
): QueryBuilder<WithResult<TContext, ResultTypeFromSelect<TSelectObject>>> {
// Check for JavaScript operators that cannot be translated to query operations
checkCallbackForJsOperators(callback)

const aliases = this._getCurrentAliases()
const refProxy = createRefProxy(aliases) as RefsForContext<TContext>
const selectObject = callback(refProxy)
Expand Down
118 changes: 118 additions & 0 deletions packages/db/src/query/builder/ref-proxy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,30 @@
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<string>,
): (hint: string) => never {
return (hint: string) => {
const pathStr = path.length > 0 ? path.join(`.`) : `<root>`
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<T = any> {
/** @internal */
readonly __refProxy: true
Expand Down Expand Up @@ -44,6 +67,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)]
Expand Down Expand Up @@ -97,6 +124,10 @@ export function createRefProxy<T extends Record<string, any>>(
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)]
Expand Down Expand Up @@ -140,6 +171,10 @@ export function createRefProxy<T extends Record<string, any>>(
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)
Expand Down Expand Up @@ -213,3 +248,86 @@ export function isRefProxy(value: any): value is RefProxy {
export function val<T>(value: T): BasicExpression<T> {
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<
T extends (...args: Array<any>) => any,
>(callback: T): 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.`,
)
}
}
}
83 changes: 83 additions & 0 deletions packages/db/tests/query/builder/ref-proxy.test.ts
Original file line number Diff line number Diff line change
@@ -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`, () => {
Expand Down Expand Up @@ -214,4 +216,85 @@ 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,
)
})
})
})
4 changes: 2 additions & 2 deletions packages/db/tests/query/live-query-collection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions packages/db/tests/query/scheduler.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
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'
Expand Down Expand Up @@ -721,7 +726,7 @@ describe(`live query scheduler`, () => {
.select(({ a, b }) => ({
id: a.id,
aValue: a.value,
bValue: b?.value ?? null,
bValue: coalesce(b!.value, null),
})),
})

Expand Down
Loading