-
Notifications
You must be signed in to change notification settings - Fork 419
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
feat(preview): add experimental support for live document id sets #7398
Open
bjoerge
wants to merge
2
commits into
preview/add-observe-full-documents
Choose a base branch
from
preview/add-live-document-id-set
base: preview/add-observe-full-documents
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(preview): add experimental support for live document id sets #7398
bjoerge
wants to merge
2
commits into
preview/add-observe-full-documents
from
preview/add-live-document-id-set
Conversation
This file contains 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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Component Testing Report Updated Aug 20, 2024 3:55 PM (UTC) ✅ All Tests Passed -- expand for details
|
bjoerge
force-pushed
the
preview/add-observe-full-documents
branch
2 times, most recently
from
August 27, 2024 09:38
5887a68
to
2897248
Compare
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
Note: builds upon the changes in #7397
This adds a new method
unstable_observeDocumentIdSet(groqFilter, params)
to the preview store that supports subscribing to a set of document ids that matches a provided groq filter. This method returns an observable that emits a new set of ids everytime a document is added or removed from the set of matching documents.Under the hood it sets up a listener for the provided filter, upon receiving the
welcome
event it fetches the initial set of ids of matching documents, and thereafter it patches the set based onappear
anddisappear
transition on the listener event.Since all we have here are document ids, and we don't now the position newly added ids, ordering can only be done based on the id itself, so this API does not include a way to specify an order with the query/filter itself.
This PR also includes a hook
useLiveDocumentSet(filter)
(also internal for now), that maps every unique id in the set to a subscription for the complete document (usingpreviewStore.observeDocument(id)
implmeneted in #7397) . This hook is using rxjs-mergemap-array which makes sure that subscriptions to individual documents are made only once, even if the referential integrity of the set itself changes, and also makes sure to unsubscribe documents that disappear from the set.What to review
Is the naming clear? I opted for "set" nomenclature to hint towards the "unordered" nature of the returned array (it is sorted by default, but consumers can also decide to add new items at the start or at the end).
Testing
These methods are only used in a feature branch for now, we'll look into some automated tests for them as we start relying on them more.
Notes for release
n/a – internal for now