-
Notifications
You must be signed in to change notification settings - Fork 48
DRAFT live query docs #242
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: main
Are you sure you want to change the base?
Conversation
|
@tanstack/db-example-react-todo
commit: |
Size Change: 0 B Total Size: 30.3 kB ℹ️ View Unchanged
|
Size Change: 0 B Total Size: 1.05 kB ℹ️ View Unchanged
|
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.
Love it!
|
||
### Convenience Function | ||
|
||
For simpler cases, you can use `createLiveQueryCollection` as a shortcut: |
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 want this? We explicitly went down the two function calls as a way of being explicit about how collections work & I'd rather keep things consistent — people can certainly add their own convenience functions if they'd like
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.
My thinking here is that this could be the exception to the rule. Users will be very regularly be creating liveQueryCollections outside of components, in loaders and other places, and if we don't provide this wrapper they will after a while get frustrated and make it themselves. The intention is that it exposes the same api as the useLiveQuery, you just use the callback to build your query, so moving between the two doesn't require any different structures in your code.
This is a strong opinion (I would expect it from the lib), but but weakly held - if you felt equally strongly that we should remove it then let's do that.
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.
yeah I don't feel that strongly against it 👍 so let's keep it
id: user.id, | ||
name: user.name, | ||
})), | ||
getKey: (user) => user.id, // Optional: uses stream key if not provided |
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.
Is there an actual use case for explicitly defining the key? We should probably then say what it is or if not, just disallow it altogether.
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.
Yes, with joins. We generate a composite key for the join using the key from each side, but often there would only need to be one side of the key.
For example if you join an aggregate query bringing in stats for "issues" - comment counts, velocity, ect. - you are joining the PK on the same PK. Issue 1 would end up with a key of [1,1]
, 2 with [2,2]
ect.
In that cases using getKey: (issue) => issue.id
would ensure that the resulting collection used the same PK as the original collection.
There is also a similar thing with groupBy, it generates a json string as the key, but often there is a better natural key that could have been used.
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.
ok, I do remember you saying this. Then yeah, adding the above as context in the docs would be great
|
||
## Basic Queries | ||
|
||
The foundation of every query is the `from` method, which specifies the source collection or subquery. You can alias the source using object syntax. |
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.
you can probably remove this section
})) | ||
``` | ||
|
||
### Query Builder Pattern |
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.
I'd expect this to show something new from the previous ones e.g. an if (filter === 'foo') { /* add a where clause */ }
) | ||
``` | ||
|
||
### Using Functions |
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 could be combined with the section above
const allPosts = createLiveQueryCollection((q) => | ||
q | ||
.from({ user: usersCollection }) | ||
.join( |
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.
hmm I kinda liked .rightJoin
more — that seems clearer than a lonely third argument
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.
Agreed, I meant to add this but it got forgotten in the mix. @thruflo was keen on a simple join()
for a left join, and we then discussed having leftJoin
, rightJoin
, innerJoin
ect. that wrapped it and set that final param.
How does that sound?
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.
👍
) | ||
``` | ||
|
||
### Conditional Query Building |
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.
redundant?
const activeUsers = createLiveQueryCollection(buildUserQuery({ activeOnly: true, limit: 10 })) | ||
``` | ||
|
||
### Caching Intermediate Results |
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.
redundant?
})) | ||
``` | ||
|
||
You can also create callbacks that take the whole row and pass them directly to `where`: |
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.
🔥
No description provided.