ReviewOS

stacks/bun-query-builder

ORM paginate(page, perPage) and query-builder paginate(perPage, page) take their arguments in opposite orders

#1092
Closed glennmichael123 opened this 22 days ago · 0 comments
22 days ago

Found on main @ 68fd61c, verified by execution.

The two paginate APIs in this library take the same two numbers in the opposite order and return differently shaped objects. Nothing errors — you get a different page than you asked for.

Repro

20 rows, ids 1–20.

// query builder
const a = await db.selectFrom('t').selectAll().paginate(5, 2)
// ids 6,7,8,9,10          -> (perPage=5, page=2)
// { data, meta: { perPage, page, total, lastPage } }

// ORM, same call
const b = await Model.query().paginate(5, 2)
// ids 9,10                -> (page=5, perPage=2)
// { data, total, page, perPage, lastPage, hasMorePages, isEmpty, from, to }

Two independent divergences:

  1. Argument order is reversed. paginate(5, 2) means "5 per page, page 2" on the builder and "page 5, 2 per page" on the ORM.
  2. Result shape differs. The builder nests under meta; the ORM is flat and carries extra fields. So page.meta.total is undefined on one of them, which at least fails loudly — unlike the argument order.

Why it matters

Both are called paginate, both take (number, number), and both are reachable from the same codebase. Moving a query from Model.query() to db.selectFrom() — or the reverse — silently changes which rows come back. There is no type error, because both parameters are number.

forPage(page, perPage) sits ~60 lines from the builder's paginate(perPage, page) in the same file and takes the opposite order again, which is presumably where the inconsistency came from.

Options

Whichever way it is resolved, it is a breaking change for one of the two call sites, so it wants a deliberate decision rather than a quiet fix:

  • Align the ORM to the builder (perPage, page) and nest under meta. Consistent with forPage being the odd one out rather than the rule.
  • Or align the builder to the ORM.
  • Or accept an options object on both (paginate({ page, perPage })) and deprecate the positional forms — the only variant that cannot be got wrong silently.

Sign in to comment on this issue.