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.

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: WebSocket is closed before the connection is established warning #8042

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {type ReactNode, useContext, useMemo} from 'react'
import {type ReactNode, useContext, useState} from 'react'
import {ResourceCacheContext} from 'sanity/_singletons'

import {createMultiKeyWeakMap, type MultiKeyWeakMap} from './createMultiKeyWeakMap'
Expand All @@ -16,7 +16,7 @@ export interface ResourceCacheProviderProps {

/** @internal */
export function ResourceCacheProvider({children}: ResourceCacheProviderProps) {
const resourceCache = useMemo((): ResourceCache => {
const [resourceCache] = useState((): ResourceCache => {
const namespaces = new Map<string, MultiKeyWeakMap>()
Copy link
Member Author

Choose a reason for hiding this comment

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

A great rule of thumb for determining wether a useMemo is safe and side-effects free is wether you can remove it and what happens to your app:

  • a) it still works, but slower
  • b) it breaks, or is so much slower it's unusable

In this case it's b), as the assumption here is that useMemo with zero dependencies means resourceCache will run exactly once, and have a stable instance for hte lifecycle of components calling it. This is false. In fact, it can choose to throw away the memoized value in certain suspense scenarios: https://react.dev/reference/react/useMemo#caveats

A better solution is to use useState, as it does have a guarantee that it's never throwing away the value, and it has a lazy init function so that our function that creates the map is still only run once.

This is also reflected in how much simpler the React Compiler optimization is for useState, compared to the useMemo.


// this is used to replace the `null` values in any `dependencies` so that
Expand All @@ -41,7 +41,7 @@ export function ResourceCacheProvider({children}: ResourceCacheProviderProps) {
namespaceMap.set(dependenciesWithoutNull, value)
},
}
}, [])
})

return (
<ResourceCacheContext.Provider value={resourceCache}>{children}</ResourceCacheContext.Provider>
Expand Down
4 changes: 2 additions & 2 deletions packages/sanity/src/core/store/_legacy/datastores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {fetchFeatureToggle} from './document/document-pair/utils/fetchFeatureTog
import {type OutOfSyncError} from './document/utils/sequentializeListenerEvents'
import {createGrantsStore, type GrantsStore} from './grants'
import {createHistoryStore, type HistoryStore} from './history'
import {__tmp_wrap_presenceStore, type PresenceStore} from './presence/presence-store'
import {createPresenceStore, type PresenceStore} from './presence/presence-store'
import {createProjectStore, type ProjectStore} from './project'
import {useResourceCache} from './ResourceCacheProvider'
import {createUserStore, type UserStore} from './user'
Expand Down Expand Up @@ -231,7 +231,7 @@ export function usePresenceStore(): PresenceStore {
resourceCache.get<PresenceStore>({
namespace: 'presenceStore',
dependencies: [bifur, connectionStatusStore, userStore],
}) || __tmp_wrap_presenceStore({bifur, connectionStatusStore, userStore})
}) || createPresenceStore({bifur, connectionStatusStore, userStore})

resourceCache.set({
namespace: 'presenceStore',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function setSessionId(id: string) {
export const SESSION_ID = getSessionId() || setSessionId(generate())

/** @internal */
export function __tmp_wrap_presenceStore(context: {
export function createPresenceStore(context: {
bifur: BifurClient
connectionStatusStore: ConnectionStatusStore
userStore: UserStore
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import {useEffect, useState} from 'react'
import {startTransition, useEffect, useReducer} from 'react'
import {useObservable} from 'react-rx'
import {of} from 'rxjs'

import {usePresenceStore} from '../datastores'
import {type DocumentPresence} from './types'

const initial: DocumentPresence[] = []
const fallback = of(initial)

/** @internal */
export function useDocumentPresence(documentId: string): DocumentPresence[] {
const presenceStore = usePresenceStore()
const [presence, setPresence] = useState<DocumentPresence[]>([])
const [mounted, mount] = useReducer(() => true, false)
// Using `startTransition` here ensures that rapid re-renders that affect the deps used by `usePresenceStore` delay the transition to `mounted=true`, thus avoiding creating websocket connections that will be closed immediately.
useEffect(() => startTransition(mount), [])

useEffect(() => {
const subscription = presenceStore.documentPresence(documentId).subscribe(setPresence)
return () => {
subscription.unsubscribe()
}
}, [documentId, presenceStore])

return presence
const presenceStore = usePresenceStore()
return useObservable(mounted ? presenceStore.documentPresence(documentId) : fallback, initial)
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import {useEffect, useState} from 'react'
import {startTransition, useEffect, useReducer} from 'react'
import {useObservable} from 'react-rx'
import {of} from 'rxjs'

import {usePresenceStore} from '../datastores'
import {type GlobalPresence} from './types'

const initial: GlobalPresence[] = []
const fallback = of(initial)

/** @internal */
export function useGlobalPresence(): GlobalPresence[] {
const [presence, setPresence] = useState<GlobalPresence[]>([])
const presenceStore = usePresenceStore()

useEffect(() => {
const subscription = presenceStore.globalPresence$.subscribe(setPresence)
const [mounted, mount] = useReducer(() => true, false)
// Using `startTransition` here ensures that rapid re-renders that affect the deps used by `usePresenceStore` delay the transition to `mounted=true`, thus avoiding creating websocket connections that will be closed immediately.
useEffect(() => startTransition(mount), [])

return () => {
subscription.unsubscribe()
}
}, [presenceStore])

return presence
const presenceStore = usePresenceStore()
return useObservable(mounted ? presenceStore.globalPresence$ : fallback, initial)
}
Loading