ReviewOS

stacks/bun-query-builder

configureOrm installs a globalDb that can never be released

#1134
Closed chris opened this 8 days ago ·
8 days ago

configureOrm() installs a globalDb that takes permanent precedence over setConfig()-based resolution for the rest of the process, and the package exports no way to release it. orm.d.ts has configureOrm, getDatabase, clearLocalModels, clearModelRegistry - nothing that undoes the first.

Why it matters

Stacks' own test suite shares one process across 815 files, and nine of them call configureOrm to point at their own temp database. Whichever runs last leaves globalDb aimed at a file its afterAll then deletes, so every later file querying through the ORM gets:

RangeError: Cannot use a closed database

Stacks has resetDatabaseConnection(), which clears both caches it owns - its own db proxy and the connection under it - and still cannot reach this one. The situation is documented in orm/tests/trait-instance-methods.test.ts:

once any test file (in-process) calls configureOrm(), its globalDb takes permanent precedence over setConfig()-based resolution for the rest of the process - no subsequent initializeDbConfig() call anywhere can undo that

Outside tests the same shape applies to anything that configures the ORM more than once in a process: a long-running worker, a CLI that switches connection, a multi-tenant request path. Today the first call wins forever.

What would help

A release, so a caller can hand globalDb back:

export function releaseOrm(): void   // or resetOrm(), clearOrm()

Semantics that would suit the callers here: drop the globalDb override so resolution falls back to setConfig(), and leave closing the underlying Database to whoever supplied it (these callers pass a path, not an instance, so the package owns that handle and should close it).

Re-pointing via configureOrm({ database: otherPath }) is a workaround, but it requires every caller to know what to point back at, which is exactly the knowledge a test file releasing its own fixture does not have.

Happy to send a PR if the shape above is roughly right - it is a small change and I have a reproduction in stacksjs/stacks#2415.

8 days ago

Resolved by commit 7e173a6 (feat: let a caller release the configureOrm override) and shipped beginning in v0.2.61. releaseOrm() clears the override, closes package-owned path-based handles, preserves caller-owned Database instances, and safely supports repeated calls. Verified on current main with the dedicated four-test regression suite and a clean typecheck.

Sign in to comment on this issue.