fix(presentation): use live content API for loaders #8429
Merged
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 ↗︎
2 Skipped Deployments
|
|
No changes to documentation |
|
Component Testing Report Updated Jan 30, 2025 3:34 PM (UTC) ❌ Failed Tests (3) -- expand for details
|
⚡️ Editor Performance ReportUpdated Thu, 30 Jan 2025 15:35:21 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
stipsan
force-pushed
the
use-live-content-api
branch
from
6bdee9b to
084ad72
Compare
stipsan
force-pushed
the
use-live-content-api
branch
from
084ad72 to
2cd609d
Compare
stipsan
force-pushed
the
use-live-content-api
branch
from
2cd609d to
39127ab
Compare
stipsan
force-pushed
the
use-live-content-api
branch
from
39127ab to
91cd5cb
Compare
stipsan
commented
Jan 29, 2025
Comment on lines
-138
to
-149
| useEffect(() => { | ||
| setOnline(navigator.onLine) | ||
| // eslint-disable-next-line @typescript-eslint/no-shadow | ||
| const online = () => setOnline(true) | ||
| const offline = () => setOnline(false) | ||
| window.addEventListener('online', online) | ||
| window.addEventListener('offline', offline) | ||
| return () => { | ||
| window.removeEventListener('online', online) | ||
| window.removeEventListener('offline', offline) | ||
| } | ||
| }, []) |
There was a problem hiding this comment.
client.live.events() handles this internally by emitting a reconnect event.
Comment on lines
-156
to
-171
| // Should pause activity when offline | ||
| if (!online) { | ||
| return true | ||
| } | ||
|
|
||
| // Should pause when the document isn't visible, as it's likely the user isn't looking at the page | ||
| if (visibilityState === 'hidden') { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| function onVisibilityChange(onStoreChange: () => void): () => void { | ||
| document.addEventListener('visibilitychange', onStoreChange) | ||
| return () => document.removeEventListener('visibilitychange', onStoreChange) |
There was a problem hiding this comment.
client.live.events() does not handle pausing on visibility hidden, but it's less necessary since we're no longer polling either. If a tab is suspended and brought back we do however handle that automatically.
Comment on lines
-114
to
-116
| const onFocus = () => setState('stale') | ||
| window.addEventListener('focus', onFocus) | ||
| return () => window.removeEventListener('focus', onFocus) |
There was a problem hiding this comment.
With client.live.events() there's no longer a need to refetch on window focus 🥳
Comment on lines
-56
to
-59
| export function useQueryParams(params?: undefined | null | QueryParams): QueryParams { | ||
| const stringifiedParams = useMemo(() => JSON.stringify(params || {}), [params]) | ||
| return useMemo(() => JSON.parse(stringifiedParams) as QueryParams, [stringifiedParams]) | ||
| } |
There was a problem hiding this comment.
The stability of query params are now handled by the useLiveQueries reducer, we no longer need this hack 😌
| @@ -227,7 +227,6 @@ | |||
| "lodash": "^4.17.21", | |||
| "log-symbols": "^2.2.0", | |||
| "mendoza": "^3.0.0", | |||
| "mnemonist": "0.39.8", | |||
There was a problem hiding this comment.
One less chonky dependency 😮💨
| }, []) | ||
|
|
||
| useEffect(() => { | ||
| useEffect((): (() => void) => { |
There was a problem hiding this comment.
This typing:
useEffect((): (() => void) => {
// ^
})Is there to avoid easy mistakes, as further down this is returned:
return nextComlink.start()Calling .start() returns an unsubscribe handler. This is a bit implicit and easy to miss. Should someone refactor this useEffect later and miss the return statement then TS will catch it 😮💨
There was a problem hiding this comment.
Nice! In future I'll explicitly call unsubscribe in cleanup functions as I'm definitely guilty of not doing this, and I can see how it could be missed in a refactor.
| key={`${key}${perspective}`} | ||
| projectId={clientConfig.projectId!} | ||
| dataset={clientConfig.dataset!} | ||
| key={`${liveEvents.resets}:${key}`} |
There was a problem hiding this comment.
The liveEvents.resets ensures that if a reconnect or restart live event happens then QuerySubscription components here start from a clean slate. This helps ensure their internal tracking of what liveEventsMessages they can ignore, as they happened before the component mounted, and which ones to look for a lastLiveEventId, works correctly.
stipsan
force-pushed
the
use-live-content-api
branch
from
91cd5cb to
9a60bd7
Compare
stipsan
commented
Jan 30, 2025
| @@ -10,26 +11,13 @@ export const EDIT_INTENT_MODE = 'presentation' | |||
| export const MAX_TIME_TO_OVERLAYS_CONNECTION = 3_000 // ms | |||
|
|
|||
| // The API version to use when using `@sanity/client` | |||
| export const API_VERSION = '2023-10-16' | |||
| export const API_VERSION = apiVersion | |||
There was a problem hiding this comment.
Aligning on the apiVersion used here increases the potential effect of sanity-io/client#990
rdunk
approved these changes
Jan 30, 2025
stipsan
added a commit
that referenced
this pull request
Jan 30, 2025
bjoerge
pushed a commit
that referenced
this pull request
Jan 30, 2025
pedrobonamin
pushed a commit
that referenced
this pull request
Feb 4, 2025
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
This PR switches Presentation when used to power loader queries (this is what
@sanity/react-loader'suseLiveModeis using) from using a aggressive polling refetch approach that fires every two seconds, to Sanity Live Content with Drafts, which ensures refetches only happen when content has actually changed.less.requests.mov
What to review
Hopefully there's enough context to understand the changes 🙌
Testing
New tests added for new state logic that are reducer based. There's still a lack of E2E tests so manual testing is required.
Notes for release
Presentation Tool is now using Sanity Live Content API to drive Live Mode in integrations like
@sanity/react-loader,@sanity/svelte-loader,@nuxtjs/sanityand@sanity/core-loader.The new setup allows query refetching to only happen when actually needed, as opposed to every two seconds.
Why have we been aggressively fetching every two seconds?
It's in order to ensure changes that can't reasonably be predicted (using Content Source Maps/CSM) client side are still caught, and eventual consistency in the live preview reached:
count(*[_type == "person" && isPublished])For the client to know when this query count will change it would need to monitor every person document and if
isPublishedis true. That doesn't scale.*[slug.current == "discounted"]If any document is given the slug
discounted, the query needs to refetch. The client would need to monitor every single document mutation in the dataset and check if a new document now has the slug, as well as check if any fields on the current matching document has changed, or if the matching document got deleted and there's a fallback match, or none at all.The gist of it is the client can't predict everything, and if it tries, it winds up using a lot of memory and perform work, just in case anything has changed.
While this over-fetching never counted towards your Sanity API hit quota, the intention was to move over to a better architecture.
And that new API is Sanity Live; it's built to solve "when should a query refetch?" and it allows Presentation to rely on the backend for telling it when it needs to refetch, thus using far less memory, less network resources, and less work on the main thread.