refactor collection for fine grained reactivity #155
Conversation
🦋 Changeset detectedLatest commit: 711c1ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
KyleAMathews
left a comment
KyleAMathews
left a comment
There was a problem hiding this comment.
very nice! I love the new Map APIs (though they should probably work on the ID not the internal key?)
|
Nice! I think the other frameworks were limited due to TS Store and this changes it. I'll update the Svelte PR and improve existing ones once this lands 🎉 |
|
@KyleAMathews I've made those changes. 100% agree on the key, the composite key should be an internal detail that isn't leaked. It would make much more sense for the internal map to just use the actual id of the collection, it makes much more sense for the public api, and then we are able to in future use it as an index on the id for faster queries that use the id. I'm not 100% sure on how you are using the composite key, I would need to look through the transaction code to understand where to make the changes. But my thinking was to just compute it on the fly when needed, rather than persist it. The other thing I wanted to suggest is that we remove the restriction on having to have a |
…balKey` for transactions, and rename getId to getKey
samwillis
force-pushed
the
samwillis/refactor-collection
branch
from
6604a33 to
bffed75
Compare
|
@tanstack/db-example-react-todo commit: |
samwillis
changed the title
samwillis
force-pushed
the
samwillis/refactor-collection
branch
from
fe422a2 to
044f46b
Compare
samwillis
changed the title
KyleAMathews
left a comment
KyleAMathews
left a comment
There was a problem hiding this comment.
Just a few questions but otherwise looks great!
|
|
||
| expect(collection.state).toEqual( | ||
| new Map([ | ||
| [`KEY::${collection.id}/1`, { id: 1, name: `Test User` }], |
|
|
||
| // Currently a query with an orderBy will add a _orderByIndex to the items | ||
| // so for now we need to sort the array by _orderByIndex if it exists | ||
| // TODO: in the future it would be much better is the keys are sorted - this |
There was a problem hiding this comment.
let's create a follow-up issue
| /** | ||
| * Subscribe to changes for a specific key | ||
| */ | ||
| public subscribeChangesKey( |
There was a problem hiding this comment.
throw error if key doesn't exist?
There was a problem hiding this comment.
Or is it useful to preemptively subscribe to a key?
There was a problem hiding this comment.
I also don't see any tests for this?
There was a problem hiding this comment.
Yep, I think preemptively subscribing to a key is useful.
I'll add tests.
| * Returns a Tanstack Store Map that is updated when the collection changes | ||
| * This is a temporary solution to enable the existing framework hooks to work | ||
| * with the new internals of Collection until they are rewritten. | ||
| * TODO: Remove this once the framework hooks are rewritten. |
There was a problem hiding this comment.
let's create follow-up issues
Co-authored-by: Kyle Mathews <mathews.kyle@gmail.com>
This is a significant refactor of the
Collectionreplacing the Store based derived state with a more fine grade system that will be faster and more efficient.Rather than computing a full "derived state" on each change, we only compute a derived upserts and deletes, along with emitting insert/update/delete messages on each change. This should result in much less object churn and far less GC.
The
Collectionis now also moreMaplike with many similar methodskeys,values,entries,get(key)andhas(key).the
config.getIdcallback has been renamedconfig.getKeyfor constancy with theMaplike interface.You can subscribe to either changes to the whole collection, or to a single key - perfect for fine grade reactivity.
Queries still "sync" into a collection as their materialised state.
Finally, as there is only loose coupling between the live query system and collections, the latter can be used without a live query if wanted.
We have temporarily added
asStoreMapandasStoreArrayto the collection to enable use with only slightly modified versions of the current frameworks hooks - this will be removed when they have been rewritten to take advantage of the frameworks own reactivity systems for fine grade reactivity.