Skip to content

Commit f691a32

Browse files
authored
Remove useSyncExternalStore from useIsDevRendering (#77651)
useIsDevRendering is a global pending state for router navigations/ actions. The idiomatic API for this pattern is useOptimistic. Previously, this was implemented using useSyncExternalStore, which led to an additional sync render every time a navigation completed. The advantage of useOptimistic is that it gets automatically reverted in the same commit as the transition it's associated with. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making:
1 parent 713caf6 commit f691a32

File tree

3 files changed

+28
-59
lines changed

3 files changed

+28
-59
lines changed

packages/next/src/client/components/react-dev-overlay/utils/dev-indicator/dev-render-indicator.tsx

+12-26
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,21 @@
33
* Used by the dev tools indicator to show render status
44
*/
55

6-
import { useSyncExternalStore } from 'react'
6+
import { useEffect, useOptimistic } from 'react'
77

8-
let isVisible = false
9-
let listeners: Array<() => void> = []
10-
11-
const subscribe = (listener: () => void) => {
12-
listeners.push(listener)
13-
return () => {
14-
listeners = listeners.filter((l) => l !== listener)
15-
}
16-
}
17-
18-
const getSnapshot = () => isVisible
19-
20-
const show = () => {
21-
isVisible = true
22-
listeners.forEach((listener) => listener())
23-
}
24-
25-
const hide = () => {
26-
isVisible = false
27-
listeners.forEach((listener) => listener())
28-
}
8+
const optimisticStateSetters = new Set<(boolean: true) => void>()
299

3010
export function useIsDevRendering() {
31-
return useSyncExternalStore(subscribe, getSnapshot)
11+
const [isDevRendering, setIsDevRendering] = useOptimistic(false)
12+
useEffect(() => {
13+
optimisticStateSetters.add(setIsDevRendering)
14+
return () => {
15+
optimisticStateSetters.delete(setIsDevRendering)
16+
}
17+
}, [setIsDevRendering])
18+
return isDevRendering
3219
}
3320

34-
export const devRenderIndicator = {
35-
show,
36-
hide,
21+
export function setDevRenderIndicatorPending() {
22+
optimisticStateSetters.forEach((setter) => setter(true))
3723
}

packages/next/src/client/components/react-dev-overlay/utils/dev-indicator/use-sync-dev-render-indicator.tsx

-16
This file was deleted.

packages/next/src/client/components/use-action-queue.ts

+16-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Dispatch } from 'react'
2-
import React, { use } from 'react'
2+
import React, { startTransition, use } from 'react'
33
import { isThenable } from '../../shared/lib/is-thenable'
44
import type { AppRouterActionQueue } from './app-router-instance'
55
import type {
@@ -19,6 +19,20 @@ export function dispatchAppRouterAction(action: ReducerActions) {
1919
'Internal Next.js error: Router action dispatched before initialization.'
2020
)
2121
}
22+
23+
if (process.env.NODE_ENV !== 'production') {
24+
// In development, set an pending state on the DevTools indicator
25+
// whenever a router action is in progress. This is backed by useOptimistic,
26+
// so we don't need to set it back to false; it will be automatically reset
27+
// when the transition completes.
28+
const setDevRenderIndicatorPending =
29+
require('./react-dev-overlay/utils/dev-indicator/dev-render-indicator')
30+
.setDevRenderIndicatorPending as typeof import('./react-dev-overlay/utils/dev-indicator/dev-render-indicator').setDevRenderIndicatorPending
31+
startTransition(() => setDevRenderIndicatorPending())
32+
}
33+
34+
// NOTE: This is wrapped in a startTransition internally, but to avoid a
35+
// refactor hazard we should consider wrapping it here instead.
2236
dispatch(action)
2337
}
2438

@@ -34,22 +48,7 @@ export function useActionQueue(
3448
// Ideally, what we'd do instead is pass the state as a prop to root.render;
3549
// this is conceptually how we're modeling the app router state, despite the
3650
// weird implementation details.
37-
if (process.env.NODE_ENV !== 'production') {
38-
const useSyncDevRenderIndicator =
39-
require('./react-dev-overlay/utils/dev-indicator/use-sync-dev-render-indicator')
40-
.useSyncDevRenderIndicator as typeof import('./react-dev-overlay/utils/dev-indicator/use-sync-dev-render-indicator').useSyncDevRenderIndicator
41-
// eslint-disable-next-line react-hooks/rules-of-hooks
42-
const syncDevRenderIndicator = useSyncDevRenderIndicator()
43-
44-
dispatch = (action: ReducerActions) => {
45-
syncDevRenderIndicator(() => {
46-
actionQueue.dispatch(action, setState)
47-
})
48-
}
49-
} else {
50-
dispatch = (action: ReducerActions) =>
51-
actionQueue.dispatch(action, setState)
52-
}
51+
dispatch = (action: ReducerActions) => actionQueue.dispatch(action, setState)
5352

5453
return isThenable(state) ? use(state) : state
5554
}

0 commit comments

Comments
 (0)