Skip to content

Commit

Permalink
remove redux devtools from router reducer (vercel#70495)
Browse files Browse the repository at this point in the history
It's a bit odd that we expose this internal debugging capability in
production and it seems to cause potential production issues when
serializing unsupported data structures ([x-ref
](vercel#69436))

If we feel a need to re-introduce the ability to introspect on the
router state even in production we could consider relanding it in an
opt-in way, and not run on every action. But I think since we've moved
away from throwing promises in reducers (back when the reducers could
potentially be replayed by React, in early Suspense implementations),
I'm not sure this provides as much value.

Fixes vercel#70441
  • Loading branch information
ztanner authored and abhi12299 committed Sep 29, 2024
1 parent a6c7cd1 commit b951c18
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 171 deletions.
16 changes: 4 additions & 12 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,7 @@ import {
PathnameContext,
PathParamsContext,
} from '../../shared/lib/hooks-client-context.shared-runtime'
import {
useReducerWithReduxDevtools,
useUnwrapState,
type ReduxDevtoolsSyncFn,
} from './use-reducer-with-devtools'
import { useReducer, useUnwrapState } from './use-reducer'
import { ErrorBoundary, type ErrorComponent } from './error-boundary'
import { isBot } from '../../shared/lib/router/utils/is-bot'
import { addBasePath } from '../add-base-path'
Expand Down Expand Up @@ -75,10 +71,8 @@ function isExternalURL(url: URL) {

function HistoryUpdater({
appRouterState,
sync,
}: {
appRouterState: AppRouterState
sync: ReduxDevtoolsSyncFn
}) {
useInsertionEffect(() => {
if (process.env.__NEXT_APP_NAV_FAIL_HANDLING) {
Expand Down Expand Up @@ -108,9 +102,7 @@ function HistoryUpdater({
} else {
window.history.replaceState(historyState, '', canonicalUrl)
}

sync(appRouterState)
}, [appRouterState, sync])
}, [appRouterState])
return null
}

Expand Down Expand Up @@ -219,7 +211,7 @@ function Router({
actionQueue: AppRouterActionQueue
assetPrefix: string
}) {
const [state, dispatch, sync] = useReducerWithReduxDevtools(actionQueue)
const [state, dispatch] = useReducer(actionQueue)
const { canonicalUrl } = useUnwrapState(state)
// Add memoized pathname/query for useSearchParams and usePathname.
const { searchParams, pathname } = useMemo(() => {
Expand Down Expand Up @@ -606,7 +598,7 @@ function Router({

return (
<>
<HistoryUpdater appRouterState={useUnwrapState(state)} sync={sync} />
<HistoryUpdater appRouterState={useUnwrapState(state)} />
<RuntimeStyles />
<PathParamsContext.Provider value={pathParams}>
<PathnameContext.Provider value={pathname}>
Expand Down
153 changes: 0 additions & 153 deletions packages/next/src/client/components/use-reducer-with-devtools.ts

This file was deleted.

35 changes: 35 additions & 0 deletions packages/next/src/client/components/use-reducer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { Dispatch } from 'react'
import React, { use } from 'react'
import { useCallback } from 'react'
import type {
AppRouterState,
ReducerActions,
ReducerState,
} from './router-reducer/router-reducer-types'
import type { AppRouterActionQueue } from '../../shared/lib/router/action-queue'
import { isThenable } from '../../shared/lib/is-thenable'

export function useUnwrapState(state: ReducerState): AppRouterState {
// reducer actions can be async, so sometimes we need to suspend until the state is resolved
if (isThenable(state)) {
const result = use(state)
return result
}

return state
}

export function useReducer(
actionQueue: AppRouterActionQueue
): [ReducerState, Dispatch<ReducerActions>] {
const [state, setState] = React.useState<ReducerState>(actionQueue.state)

const dispatch = useCallback(
(action: ReducerActions) => {
actionQueue.dispatch(action, setState)
},
[actionQueue]
)

return [state, dispatch]
}
6 changes: 0 additions & 6 deletions packages/next/src/shared/lib/router/action-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
ACTION_NAVIGATE,
ACTION_RESTORE,
} from '../../../client/components/router-reducer/router-reducer-types'
import type { ReduxDevToolsInstance } from '../../../client/components/use-reducer-with-devtools'
import { reducer } from '../../../client/components/router-reducer/router-reducer'
import { startTransition } from 'react'
import { isThenable } from '../is-thenable'
Expand All @@ -16,7 +15,6 @@ export type DispatchStatePromise = React.Dispatch<ReducerState>

export type AppRouterActionQueue = {
state: AppRouterState
devToolsInstance?: ReduxDevToolsInstance
dispatch: (payload: ReducerActions, setState: DispatchStatePromise) => void
action: (state: AppRouterState, action: ReducerActions) => ReducerState
pending: ActionQueueNode | null
Expand Down Expand Up @@ -85,10 +83,6 @@ async function runAction({

actionQueue.state = nextState

if (actionQueue.devToolsInstance) {
actionQueue.devToolsInstance.send(payload, nextState)
}

runRemainingActions(actionQueue, setState)
action.resolve(nextState)
}
Expand Down

0 comments on commit b951c18

Please sign in to comment.