ReviewOS

also looking at this

stacks/bun-query-builder

fix(browser)!: refuse orWhere rather than sending it as an AND

#1131
Merged glennmichael123 wants to merge fix/1096-browser-orwhere into main
2 files +136 -34
9 days ago

Closes #1096 — the half left open by #1108, which fixed not in.

Both browser builders recorded boolean: 'or' on the where list, and buildQueryParams never read it. Every recorded where was appended as one more parameter, which any receiving API reads as a conjunction:

Model.query().where('role', 'admin').orWhere('role', 'moderator')
// role=admin&role=moderator        AND, not OR

The caller gets a superset of the rows they asked for — no error, and a request that looks correct in a network panel.

Why throw rather than invent a spelling

The other option was serialising as filter[col][or][]=…, matching how is / is not already get distinct spellings. But that has to be agreed with whatever serves these requests, and I don't know that contract. Shipping a format unilaterally reproduces exactly the same silent failure one layer further away: the server ignores what it doesn't recognise, and the OR term vanishes just as quietly as it does today.

Throwing puts the failure at the call site, where the person who can fix it is standing.

The message names the mechanism and leaves somewhere to go:

[browser] orWhere() is not supported by this client. The query-param format it builds has no spelling for a disjunction, so the term would be sent as a plain filter and the server would read it as AND — silently returning a superset of the rows you asked for. Express the alternatives as a single filter (whereIn), or issue one request per branch and merge the results.

A refusal that doesn't say why reads as arbitrary, and the caller can't tell whether it's a bug.

This is a break — deliberately

Marked !. Anyone calling orWhere on these builders today starts getting a TypeError.

What they have today is already wrong, just quietly, so the trade is bad rows for a message. Nothing in the test suite covered it, and the server-side query builder's orWhere is entirely unaffected — that one composes real SQL and got its own grouping fix in #1083.

Verification

7 new tests: both builders throw, it's a TypeError (matching the other refusals in this codebase), the message contains the mechanism and the suggested alternative, and nothing is sent — the throw happens at the call, not at the request, verified against a captured fetch.

Two guard tests pin the conjunctive paths as untouched: chained where() still serialises as before, and whereIn — the replacement the message points at — still emits role[]=admin,moderator.

5 of the 7 fail without the change. Full suite 2123 pass / 0 fail, typecheck clean, pickier 0 errors.

🤖 Generated with Claude Code

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.