-
Notifications
You must be signed in to change notification settings - Fork 669
Add a typed query builder for the typescript client #4021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…different packages/examples
joshua-spacetime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have tests for the typescript sdk that we could potentially extend to exercise the query builder?
| on: ( | ||
| left: IndexedRowExpr<TableDef>, | ||
| right: IndexedRowExpr<RightTable> | ||
| ) => EqExpr<TableDef | RightTable> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still too permissive, right? Is it possible to restrict it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are you concerned about this allowing? The return type of EqExpr doesn't enforce that columns are indexed, but the left and right parameters only have indexed columns, so anyone constructing queries the normal way won't run into issues.
|
I added another version of the query builder tests that uses the test-app bindings in client_query.test.ts |
Description of Changes
This moves the query builder code out of the
serverpackage and intolibso it can be shared by the client and server.I put the query builder in the
index.tsof module bindings as aqueryobject that can be imported. The typescripttest-apphas an example of using it with the subscription builder.This is branched off of
https://github.com/clockworklabs/SpacetimeDB/pull/3980.API and ABI breaking changes
This extends the client subscription builder API to allow
string | RowTypedQuery<any, any>, so existing client code should be fine.Expected complexity level and risk
1.5. This is low risk.
Testing
I manually tested that the test app still works locally.