Repository navigation
Type the tables in the subscribe callback - #6358
Open
cloutiertyler wants to merge 6 commits into
Open
cloutiertyler wants to merge 6 commits into
cloutiertyler wants to merge 6 commits into
Conversation
`NamespacedQueryBuilder<RemoteModule & UntypedSchemaDef>` merged `Record<string, UntypedTableDef>` into the module's tables, so every entry in the callback was an untyped table ref: unknown tables and columns compiled, and a view with args wasn't callable without a cast. The callback now takes `NamespacedQueryBuilder<RemoteModule>`. A submodule's table, which generated bindings give a dotted accessor name, stays reachable under its namespace as `makeFromBuilder` puts it there, untyped as before, since its key is the rest of its `sourceName`, which the types don't record. `subscription_builder.test-d.ts` checks this in `pnpm build`.
`makeFromBuilder` grouped a submodule's table by its dotted `sourceName`, so `lib.peopleBy` was `tables.lib.people_by`, unlike `conn.db`, reducers and procedures, which #6089 nested by accessor name. It now nests with `accessorSlot` and is typed with `NestAccessors`, as `conn.db` is, so namespaced tables are typed too. The `namespaces` branch of `NamespacedQueryBuilder` is gone: client bindings never declare `namespaces`, and `makeFromBuilder` never read it.
…e raw def's sourceName
…ng sourceName as a deprecated alias
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
Stacked on #6336, this PR types the
subscribe(tables => ...)callback, which intersecting the module withUntypedSchemaDefhad erased to untyped table refs since #5486 (hence the cast #6336's description recommends for view calls), keys its submodule tables by accessor name as #6089 did forconn.db, and renames thesourceNameof table defs and table refs totableName, since it holds the name SQL uses while the raw def'ssourceNameis the accessor name.API and ABI breaking changes
Yes, but only for submodule tables inside the callback, which were keyed by the rest of their canonical name (
tables.lib.people_by) and are now keyed by accessor name (tables.lib.peopleBy) likeconn.db; that only differs when the two names differ, and code using the old key now fails to compile rather than silently working. Otherwise, only code that can't work today breaks: a callback that names a missing table or column, or passes a mistyped value, now fails to compile instead of failing when it subscribes. The rename doesn't break anything, becausesourceNamestays as a deprecated alias oftableNameand generated bindings never read it.Rollback safety impact
n/a
Expected complexity level and risk
1
Testing
src/sdk/subscription_builder.test-d.ts, whichpnpm buildtypechecks, fails on the old code and passes with the fixmakeFromBuildertests intests/query.test.tsfor accessor-name keystests/query.test.tsthatsourceNamestill works as an alias, which fails without the aliaspnpm build,pnpm test:unit(except the two wasm test-utils files, which need the Rust build), eslint, and prettier