Skip to content

Commit

Permalink
suspend in render, not in reducers (#56497)
Browse files Browse the repository at this point in the history
This removes our current convention of throwing promises in reducers in
favor of returning promises that can be consumed by `use` instead. This
will help unblock some future improvements (batching, PPR)

Reducers that would typically throw a promise now return their promise.
This gets maintained by a mutable queue (initialized in hydrate) to
ensure actions are processed in-order. The queue is also responsible for
mutating state and passing it as an input to subsequent actions.

This PR does not modify reducer behavior to keep changes minimal, but
there's more cleanup that we can do in a follow-up PR to remove things
that previously assumed reducers would be replayed.

(I recommend reviewing with whitespace turned off)

---------

Co-authored-by: Tim Neutkens <[email protected]>
  • Loading branch information
ztanner and timneutkens authored Nov 2, 2023
1 parent be61804 commit 85abc48
Show file tree
Hide file tree
Showing 16 changed files with 720 additions and 566 deletions.
14 changes: 11 additions & 3 deletions packages/next/src/client/app-index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { GlobalLayoutRouterContext } from '../shared/lib/app-router-context.shar
import onRecoverableError from './on-recoverable-error'
import { callServer } from './app-call-server'
import { isNextRouterError } from './components/is-next-router-error'
import {
ActionQueueContext,
createMutableActionQueue,
} from '../shared/lib/router/action-queue'

// Since React doesn't call onerror for errors caught in error boundaries.
const origConsoleError = window.console.error
Expand Down Expand Up @@ -222,16 +226,20 @@ export function hydrate() {
}
}

const actionQueue = createMutableActionQueue()

const reactEl = (
<StrictModeIfEnabled>
<HeadManagerContext.Provider
value={{
appDir: true,
}}
>
<Root>
<RSCComponent />
</Root>
<ActionQueueContext.Provider value={actionQueue}>
<Root>
<RSCComponent />
</Root>
</ActionQueueContext.Provider>
</HeadManagerContext.Provider>
</StrictModeIfEnabled>
)
Expand Down
55 changes: 24 additions & 31 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import type {
FlightData,
} from '../../server/app-render/types'
import type { ErrorComponent } from './error-boundary'
import { reducer } from './router-reducer/router-reducer'
import {
ACTION_FAST_REFRESH,
ACTION_NAVIGATE,
Expand All @@ -36,7 +35,6 @@ import {
PrefetchKind,
} from './router-reducer/router-reducer-types'
import type {
Mutable,
ReducerActions,
RouterChangeByServerResponse,
RouterNavigate,
Expand All @@ -47,7 +45,10 @@ import {
SearchParamsContext,
PathnameContext,
} from '../../shared/lib/hooks-client-context.shared-runtime'
import { useReducerWithReduxDevtools } from './use-reducer-with-devtools'
import {
useReducerWithReduxDevtools,
useUnwrapState,
} from './use-reducer-with-devtools'
import { ErrorBoundary } from './error-boundary'
import { createInitialRouterState } from './router-reducer/create-initial-router-state'
import type { InitialRouterStateParameters } from './router-reducer/create-initial-router-state'
Expand All @@ -73,9 +74,9 @@ export function getServerActionDispatcher() {
return globalServerActionDispatcher
}

let globalMutable: Mutable['globalMutable'] = {
refresh: () => {}, // noop until the router is initialized
}
const globalMutable: {
pendingMpaPath?: string
} = {}

export function urlToUrlWithoutFlightMarker(url: string): URL {
const urlWithoutFlightParameters = new URL(url, location.origin)
Expand Down Expand Up @@ -131,7 +132,7 @@ function HistoryUpdater({ tree, pushRef, canonicalUrl, sync }: any) {
return null
}

const createEmptyCacheNode = () => ({
export const createEmptyCacheNode = () => ({
status: CacheStates.LAZY_INITIALIZED,
data: null,
subTreeData: null,
Expand All @@ -145,7 +146,7 @@ function useServerActionDispatcher(dispatch: React.Dispatch<ReducerActions>) {
dispatch({
...actionPayload,
type: ACTION_SERVER_ACTION,
mutable: { globalMutable },
mutable: {},
cache: createEmptyCacheNode(),
})
})
Expand Down Expand Up @@ -174,7 +175,7 @@ function useChangeByServerResponse(
previousTree,
overrideCanonicalUrl,
cache: createEmptyCacheNode(),
mutable: { globalMutable },
mutable: {},
})
})
},
Expand All @@ -186,7 +187,6 @@ function useNavigate(dispatch: React.Dispatch<ReducerActions>): RouterNavigate {
return useCallback(
(href, navigateType, forceOptimisticNavigation, shouldScroll) => {
const url = new URL(addBasePath(href), location.href)
globalMutable.pendingNavigatePath = createHrefFromUrl(url)

return dispatch({
type: ACTION_NAVIGATE,
Expand All @@ -197,7 +197,7 @@ function useNavigate(dispatch: React.Dispatch<ReducerActions>): RouterNavigate {
shouldScroll: shouldScroll ?? true,
navigateType,
cache: createEmptyCacheNode(),
mutable: { globalMutable },
mutable: {},
})
},
[dispatch]
Expand Down Expand Up @@ -229,25 +229,15 @@ function Router({
}),
[buildId, children, initialCanonicalUrl, initialTree, initialHead]
)
const [
{
tree,
cache,
prefetchCache,
pushRef,
focusAndScrollRef,
canonicalUrl,
nextUrl,
},
dispatch,
sync,
] = useReducerWithReduxDevtools(reducer, initialState)
const [reducerState, dispatch, sync] =
useReducerWithReduxDevtools(initialState)

useEffect(() => {
// Ensure initialParallelRoutes is cleaned up from memory once it's used.
initialParallelRoutes = null!
}, [])

const { canonicalUrl } = useUnwrapState(reducerState)
// Add memoized pathname/query for useSearchParams and usePathname.
const { searchParams, pathname } = useMemo(() => {
const url = new URL(
Expand Down Expand Up @@ -322,7 +312,7 @@ function Router({
dispatch({
type: ACTION_REFRESH,
cache: createEmptyCacheNode(),
mutable: { globalMutable },
mutable: {},
origin: window.location.origin,
})
})
Expand All @@ -338,7 +328,7 @@ function Router({
dispatch({
type: ACTION_FAST_REFRESH,
cache: createEmptyCacheNode(),
mutable: { globalMutable },
mutable: {},
origin: window.location.origin,
})
})
Expand All @@ -356,11 +346,10 @@ function Router({
}
}, [appRouter])

useEffect(() => {
globalMutable.refresh = appRouter.refresh
}, [appRouter.refresh])

if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line react-hooks/rules-of-hooks
const { cache, prefetchCache, tree } = useUnwrapState(reducerState)

// This hook is in a conditional but that is ok because `process.env.NODE_ENV` never changes
// eslint-disable-next-line react-hooks/rules-of-hooks
useEffect(() => {
Expand Down Expand Up @@ -408,6 +397,7 @@ function Router({
// probably safe because we know this is a singleton component and it's never
// in <Offscreen>. At least I hope so. (It will run twice in dev strict mode,
// but that's... fine?)
const { pushRef } = useUnwrapState(reducerState)
if (pushRef.mpaNavigation) {
// if there's a re-render, we don't want to trigger another redirect if one is already in flight to the same URL
if (globalMutable.pendingMpaPath !== canonicalUrl) {
Expand Down Expand Up @@ -466,6 +456,9 @@ function Router({
}
}, [onPopState])

const { cache, tree, nextUrl, focusAndScrollRef } =
useUnwrapState(reducerState)

const head = useMemo(() => {
return findHeadInCache(cache, tree[1])
}, [cache, tree])
Expand Down Expand Up @@ -513,7 +506,7 @@ function Router({
<LayoutRouterContext.Provider
value={{
childNodes: cache.parallelRoutes,
tree: tree,
tree,
// Root node always has `url`
// Provided in AppTreeContext to ensure it can be overwritten in layout-router
url: canonicalUrl,
Expand Down

This file was deleted.

This file was deleted.

Loading

0 comments on commit 85abc48

Please sign in to comment.