Skip to content
Merged
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
67 changes: 51 additions & 16 deletions crates/bindings-typescript/src/lib/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,15 +710,38 @@ type BooleanExprData<Table extends TypedTableDef> = (
_tableType?: Table;
};

type AndOrMixedTableScopeError = {
readonly 'Cannot combine predicates from different table scopes with and/or. In semijoin on(...), keep only the join equality and move extra predicates to .where(...).': never;
};

type RequireSameAndOrTable<
Expected extends TypedTableDef,
Actual extends TypedTableDef,
> = [Expected] extends [Actual]
? [Actual] extends [Expected]
? unknown
: AndOrMixedTableScopeError
: AndOrMixedTableScopeError;

export class BooleanExpr<Table extends TypedTableDef> {
constructor(readonly data: BooleanExprData<Table>) {}

and(other: BooleanExpr<Table>): BooleanExpr<Table> {
return new BooleanExpr({ type: 'and', clauses: [this.data, other.data] });
and<OtherTable extends TypedTableDef>(
other: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>
): BooleanExpr<Table> {
return new BooleanExpr({
type: 'and',
clauses: [this.data, other.data as BooleanExprData<Table>],
});
}

or(other: BooleanExpr<Table>): BooleanExpr<Table> {
return new BooleanExpr({ type: 'or', clauses: [this.data, other.data] });
or<OtherTable extends TypedTableDef>(
other: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>
): BooleanExpr<Table> {
return new BooleanExpr({
type: 'or',
clauses: [this.data, other.data as BooleanExprData<Table>],
});
}

not(): BooleanExpr<Table> {
Expand All @@ -732,28 +755,40 @@ export function not<T extends TypedTableDef>(
return new BooleanExpr({ type: 'not', clause: clause.data });
}

export function and<T extends TypedTableDef>(
...clauses: readonly [BooleanExpr<T>, BooleanExpr<T>, ...BooleanExpr<T>[]]
): BooleanExpr<T> {
export function and<
Table extends TypedTableDef,
OtherTable extends TypedTableDef,
>(
first: BooleanExpr<Table>,
second: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>,
...rest: readonly BooleanExpr<Table>[]
): BooleanExpr<Table> {
const clauses = [first, second, ...rest];
return new BooleanExpr({
type: 'and',
clauses: clauses.map(c => c.data) as [
BooleanExprData<T>,
BooleanExprData<T>,
...BooleanExprData<T>[],
BooleanExprData<Table>,
BooleanExprData<Table>,
...BooleanExprData<Table>[],
],
});
}

export function or<T extends TypedTableDef>(
...clauses: readonly [BooleanExpr<T>, BooleanExpr<T>, ...BooleanExpr<T>[]]
): BooleanExpr<T> {
export function or<
Table extends TypedTableDef,
OtherTable extends TypedTableDef,
>(
first: BooleanExpr<Table>,
second: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>,
...rest: readonly BooleanExpr<Table>[]
): BooleanExpr<Table> {
const clauses = [first, second, ...rest];
return new BooleanExpr({
type: 'or',
clauses: clauses.map(c => c.data) as [
BooleanExprData<T>,
BooleanExprData<T>,
...BooleanExprData<T>[],
BooleanExprData<Table>,
BooleanExprData<Table>,
...BooleanExprData<Table>[],
],
});
}
Expand Down
4 changes: 4 additions & 0 deletions crates/bindings-typescript/src/server/view.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ spacetime.anonymousView({ name: 'v5', public: true }, arrayRetValue, ctx => {
.where(row => row.id.eq(5))
.leftSemijoin(ctx.from.order, (p, o) => p.name.eq(o.person_name))
.build();
const _mixedScopeAndInJoinPredicate = ctx.from.person
// @ts-expect-error semijoin on(...) only supports one table scope for and/or clauses.
.leftSemijoin(ctx.from.order, (p, o) => p.id.eq(o.id).and(o.id.eq(5)))
.build();
return ctx.from.person
.where(row => row.id.eq(5))
.leftSemijoin(ctx.from.order, (p, o) => p.id.eq(o.id))
Expand Down
87 changes: 87 additions & 0 deletions crates/bindings-typescript/tests/query_error_message.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import * as ts from 'typescript';
import { describe, expect, it } from 'vitest';

const bindingsRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
'..'
);

function runTypecheck(semijoinPredicateExpr: string) {
const tmpDir = mkdtempSync(path.join(tmpdir(), 'stdb-query-diag-'));
const reproPath = path.join(tmpDir, 'repro.ts');

const imports = {
query: path.join(bindingsRoot, 'src/lib/query.ts'),
moduleBindings: path.join(
bindingsRoot,
'test-app/src/module_bindings/index.ts'
),
sys: path.join(bindingsRoot, 'src/server/sys.d.ts'),
};

const source = `
import { and } from ${JSON.stringify(imports.query)};
import { tables } from ${JSON.stringify(imports.moduleBindings)};

tables.player
.leftSemijoin(tables.unindexed_player, (l, r) => ${semijoinPredicateExpr})
.build();
`;

writeFileSync(reproPath, source);

try {
const options: ts.CompilerOptions = {
target: ts.ScriptTarget.ESNext,
module: ts.ModuleKind.ESNext,
strict: true,
noEmit: true,
skipLibCheck: true,
forceConsistentCasingInFileNames: true,
allowImportingTsExtensions: true,
noImplicitAny: true,
moduleResolution: ts.ModuleResolutionKind.Bundler,
useDefineForClassFields: true,
verbatimModuleSyntax: true,
isolatedModules: true,
};

const host = ts.createCompilerHost(options);
const program = ts.createProgram([reproPath, imports.sys], options, host);
const diagnostics = ts.getPreEmitDiagnostics(program);
const output = diagnostics
.map(d => ts.flattenDiagnosticMessageText(d.messageText, '\n'))
.join('\n');

return {
status: diagnostics.length === 0 ? 0 : 1,
output,
};
} finally {
rmSync(tmpDir, { recursive: true, force: true });
}
}

describe('query builder diagnostics', () => {
const messageStart =
'Cannot combine predicates from different table scopes with and/or.';
const messageHint = 'move extra predicates to .where(...)';

it('reports a clear message for free-floating and(...) in semijoin predicates', () => {
const { status, output } = runTypecheck('and(l.id.eq(r.id), r.id.eq(5))');
expect(status).not.toBe(0);
expect(output).toContain(messageStart);
expect(output).toContain(messageHint);
});

it('reports a clear message for method-style .and(...) in semijoin predicates', () => {
const { status, output } = runTypecheck('l.id.eq(r.id).and(r.id.eq(5))');
expect(status).not.toBe(0);
expect(output).toContain(messageStart);
expect(output).toContain(messageHint);
});
});
Loading