Skip to content

fix(querybuilder): type several table options in one pipe call - #36

Merged
relsunkaev merged 3 commits into
relsunkaev:mainfrom
czxtm:fix/table-pipe-multiple-options
Nov 3, 2026
Merged

relsunkaev merged 3 commits into
relsunkaev:mainfrom
czxtm:fix/table-pipe-multiple-options

Conversation

@czxtm

@czxtm czxtm commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Closes #35

The README shows examples that use .pipe(q1, q2, ...) but that loses its types. It only keeps its types if you do .pipe(q1).pipe(q2).pipe(q3) which I think we can both agree is comparatively ugly.

Why

Problem: the README's memberships table loses its types. It pipes five options in one call, which leaves every column as BoundColumn<any, ...> | undefined. Queries over that table then fail with "excessively deep" errors, and Query.ResultRow falls back to an index signature. Chaining one option per pipe call keeps the types.

  • The table pipe overload in TableOptionPipe accepts one option. With more, the call falls through to Effect's generic Pipeable.pipe.
  • Pipeable.pipe cannot carry a table option, which is a generic function that needs the table as context. Each option's callback gets no table, and the result widens to the generic table type.
  • The README type tests compile the example but never read a column afterwards, so nothing caught this.

Key Changes

Table pipe overloads (internal/table.ts):

  • Add overloads for two to nine options. Each option is typed against the table it is piped onto, so its callbacks see real columns, and each keeps its validation argument as a single option does.
  • Compute the resulting table from all options at once (ApplyTableOptions: the last primary key and every option's conflict arbiters). Nesting ApplyOption made TypeScript unable to show that tables stay assignable to the generic table type.

Type tests:

  • table-option-pipe-types.ts checks that one call with several options produces exactly the same table type as one call per option, for five options and for nine, and that columns and result rows keep their types.
  • Two ddl-types.ts checks passed only because their multi-option table had widened, which let an index on a "missing" column through. They now expect the error.

A table piped with more than one option, as the README declares
`memberships`, lost its column types: its columns became
`BoundColumn<any, ...> | undefined`. The table-specific `pipe` overload
accepts one option, so several options fell through to Effect's generic
`Pipeable.pipe`. That signature cannot carry a table option, which is a
generic function that needs the table as context, so each option's
callback lost its table and the result widened to the generic table.

Add overloads for two to nine options. Each option is typed against the
table it is piped onto, keeps its validation argument, and the resulting
table is computed from all of them at once rather than by nesting
`ApplyOption`, which kept tables assignable to the generic table type.

The new type tests check that one call with several options produces the
same table type as one call per option. Two ddl type tests used a
multi-option table whose columns had widened, so an index on a missing
column was accepted; they now expect the error.
@czxtm
czxtm requested a review from relsunkaev as a code owner October 31, 2026 16:22
Effect 4.1.0 replaced `effect/Crypto` with the `effect/crypto/*`
modules. effect-db still imports `effect/Crypto`, so a consumer that
resolves the `^4.0.0` peer to 4.1 fails to load `effect-db/postgres/migrate`
with ERR_MODULE_NOT_FOUND. The packed smoke test now fails on main for
this reason.

Cap both packages' Effect peer at `>=4.0.0 <4.1.0`. Also pin
`@effect/platform-node-shared` to 4.0.0 in effect-db, alongside its other
pinned Effect packages: `@effect/platform-node@4.0.0` accepts `^4.0.0`
for it, which resolves to 4.1.1, and that release requires Effect ^4.1.1.

Supporting Effect 4.1 means moving effect-db to the new crypto modules,
which changes the `Crypto` requirement in its public `*Effect` signatures.
@czxtm

czxtm commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

FYI, this lib is currently incompatible with effect 4.1.0 or higher due to the crypto changes. im going to set a max version on my pr so that CI will succeed when you run it. current main most likely will fail in its current state.

Effect 4.0.3 changed `Crypto.randomUUIDv4` from an Effect into a method.
Source discovery yielded it as an Effect, so `effectdb push` failed under
any Effect from 4.0.3 with "yield* ... is not iterable". The packed CLI
live smoke installs the newest Effect below 4.1 and hit this.

The UUID only makes the temporary discovery module's file name unique,
so take it from the platform's Web Crypto, which Node 22+ provides. The
migration checksum's `Crypto.digest` is unchanged across 4.0.x.
@relsunkaev
relsunkaev merged commit 25b6885 into relsunkaev:main Nov 3, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

README example is inaccurate

2 participants