ReviewOS

also looking at this

stacks/bun-query-builder

fix(types)!: type defineModel as the ORM model it returns

#1150
Merged glennmichael123 wants to merge fix/define-model-query-types into main
5 files +288 -7
9 days ago

Follow-up to #1148, from the review of that fix.

defineModel() was typed as the wrong model

The package root runs on Bun, where defineModel() returns the ORM model from createModel. Its type said browser model, so the two disagreed in both directions:

  • Worked, did not type-check: whereRaw, with, whereGroup, increment, chunk, the aggregates — everything the ORM builder adds. #1148 documented User.query().whereRaw(sql, ...bindings), which was a TS2339 for anyone whose models come from defineModel.
  • Type-checked, then threw: Model.update(id, data), Model.delete(id), and query().find(), findOrFail(), latest(), oldest() — all TypeError: ... is not a function, because the ORM had none of them.

defineModel is now typed as ModelStatic<TDef> plus the raw definition build tools read, and the methods in the second list were added to the ORM so those calls keep compiling and start working. Browser code builds its models with createBrowserModel from bun-query-builder/browser, as Stacks already does.

The definition keeps its narrow column and relation names: a literal definition normally satisfies the ORM's ModelDefinition already, and the type only intersects when it does not.

The new ORM methods

query().find(id) / findOrFail(id)the row with that primary key within the query's other constraints, including its soft-delete scope
query().latest(col?) / oldest(col?)order by created_at, or the given column, like the statics of the same name
Model.update(id, data)fill, timestamps and hooks, same as instance.update(); throws when there is no such row
Model.delete(id)goes through instance.delete(), so a soft-deletable model is soft-deleted and delete hooks run; false when there is no such row

Model.destroy(id) and remove(id) still hard-delete and skip hooks. That is unchanged here and worth a separate look.

Breaking

The root ModelQueryBuilder type is now the ORM builder. It was export type { ModelQueryBuilder } from './dynamodb', so import type { ModelQueryBuilder } from 'bun-query-builder' gave the DynamoDB interface while createModel(...).query() returned something else entirely. The DynamoDB interface is still ModelQueryBuilder in bun-query-builder/dynamodb, and is now also exported from the root as DynamoDBModelQueryBuilder. In stacksjs/stacks the name is only re-exported, never used.

Code that calls the browser-model methods against a server model keeps compiling only where the ORM now has them, which is every method listed above. Anything else typed off the browser model (for example a query() chain method the ORM does not have) becomes a type error instead of a runtime throw.

Tests

  • test/orm.define-model-server-api.test.ts — 12 tests: find within constraints and under the soft-delete scope, findOrFail messages, latest/oldest by default and explicit column, update and its throw, delete hard, soft and missing, delete hooks, and whereRaw/whereGroup/increment on a defineModel model. 10 of the 12 fail against main.
  • src/__tests__/type-usage-compile.ts — the ORM methods compile off defineModel, unknown columns are still rejected, and the two root builder exports resolve to the right interfaces. Without the type fix these produce 10 errors.

Locally: full suite 2230 pass / 0 fail / 10 skip, root and package tsc clean, pickier . at its 330-warning baseline, bun run build emits the new signatures and public-surface-types passes.

🤖 Generated with Claude Code

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

Sign in to comment on this pull request.