Skip to content
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.

Sign up for GitHub

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

fix(core): avoid unnecessary re-renders in useDocumentPresence #7365

Merged
merged 4 commits into from
Aug 22, 2024
Merged

fix(core): avoid unnecessary re-renders in useDocumentPresence #7365

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08fe788
fix(core): avoid unnecessary re-renders in useDocumentPresence
runeb Aug 13, 2024
195016f
fix(core): make sure useDocumentPresence only emits when there are ch…
runeb Aug 13, 2024
a9815f7
Merge branch 'next' into fix/useDocumentPresence-state-re-renders
runeb Aug 21, 2024
5fc1e1b
Merge branch 'next' into fix/useDocumentPresence-state-re-renders
runeb Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 28 additions & 23 deletions packages/sanity/src/core/store/_legacy/presence/presence-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

import {type BifurClient} from '@sanity/bifur-client'
import {type User} from '@sanity/types'
import {flatten, groupBy, omit, uniq} from 'lodash'
import {flatten, groupBy, isEqual, omit, uniq} from 'lodash'
import {nanoid} from 'nanoid'
import {
BehaviorSubject,
combineLatest,
defer,
EMPTY,
from,
Expand All @@ -19,7 +20,6 @@ import {
auditTime,
distinctUntilChanged,
filter,
flatMap,
map,
mergeMapTo,
scan,
Expand All @@ -29,7 +29,6 @@ import {
switchMapTo,
takeUntil,
tap,
toArray,
withLatestFrom,
} from 'rxjs/operators'

Expand Down Expand Up @@ -292,29 +291,35 @@ export function __tmp_wrap_presenceStore(context: {
),
)

// Create a single shared observable for all documents
const allDocumentsPresence$ = combineLatest([allSessions$, debugIntrospect$]).pipe(
map(([userAndSessions, debugIntrospect]) =>
userAndSessions
.flatMap((userAndSession) =>
(userAndSession.session.locations || []).map((location) => ({
documentId: location.documentId,
presence: {
user: userAndSession.user,
lastActiveAt: userAndSession.session.lastActiveAt,
path: location.path || [],
sessionId: userAndSession.session.sessionId,
selection: location?.selection,
},
})),
)
.filter((item) => debugIntrospect || item.presence.sessionId !== SESSION_ID),
),
shareReplay(1),
Copy link
Member Author

@runeb runeb Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we can eliminate shareReplay this as its introducing new behavior. Thought was that new subscribers could get the latest event sent to them when they subscribe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! allSessions$ already replays it's latest value to new subscribers, but this also shares the mapping work done above between all subscribers 👌🏼

)

// export
const documentPresence = (documentId: string): Observable<DocumentPresence[]> => {
return allSessions$.pipe(
withLatestFrom(debugIntrospect$),
switchMap(([userAndSessions, debugIntrospect]) =>
from(userAndSessions).pipe(
filter(
(userAndSession) => debugIntrospect || userAndSession.session.sessionId !== SESSION_ID,
),
flatMap((userAndSession) =>
(userAndSession.session.locations || [])
.filter((item) => item.documentId === documentId)
.map((location) => ({
user: userAndSession.user,
lastActiveAt: userAndSession.session.lastActiveAt,
path: location.path || [],
sessionId: userAndSession.session.sessionId,
selection: location?.selection,
})),
),
toArray(),
),
return allDocumentsPresence$.pipe(
map((allPresence) =>
allPresence.filter((item) => item.documentId === documentId).map((item) => item.presence),
),
// Only emit if the presence has changed for this document id
distinctUntilChanged((prev, curr) => isEqual(prev, curr)),
)
}

Expand Down
Loading