Skip to content

[dev-tools] refactor: calculate dev indicator state from the server #79557

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

Draft
wants to merge 2 commits into
base: jiwon/05-23-_dev-tools_refactor_merge_devtools-related_states_into_single_state
Choose a base branch
from
Draft
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
66 changes: 29 additions & 37 deletions packages/next/src/client/components/react-dev-overlay/shared.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { useReducer } from 'react'

import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
import type { VersionInfo } from '../../../server/dev/parse-version-info'
import type { SupportedErrorEvent } from './ui/container/runtime-error/render-error'
import type { ComponentStackFrame } from './utils/parse-component-stack'
import type { DebugInfo } from './types'
import type { DevToolsServerState } from '../../../server/dev/dev-tools-server-state'
import type { HMR_ACTION_TYPES } from '../../../server/dev/hot-reloader-types'
import { getOwnerStack } from '../errors/stitched-error'
Expand All @@ -15,13 +13,10 @@ type FastRefreshState =
/** The refresh process has been triggered, but the new code has not been executed yet. */
| { type: 'pending'; errors: SupportedErrorEvent[] }

export type DevToolsClientState = {
versionInfo: VersionInfo
debugInfo: DebugInfo
devIndicator: {
export type DevToolsClientState = DevToolsServerState & {
devIndicator: DevToolsServerState['devIndicator'] & {
staticIndicator: boolean
showIndicator: boolean
disableDevIndicator: boolean
isReady: boolean
}
}

Expand Down Expand Up @@ -125,9 +120,6 @@ function pushErrorFilterDuplicates(
return pendingErrors
}

const shouldDisableDevIndicator =
process.env.__NEXT_DEV_INDICATOR?.toString() === 'false'

export const INITIAL_OVERLAY_STATE: Omit<OverlayState, 'routerType'> = {
nextId: 1,
buildError: null,
Expand All @@ -139,13 +131,12 @@ export const INITIAL_OVERLAY_STATE: Omit<OverlayState, 'routerType'> = {
debugInfo: { devtoolsFrontendUrl: undefined },
devIndicator: {
staticIndicator: false,
disableDevIndicator: false,
/*
This is set to `true` when we can reliably know
whether the indicator is in disabled state or not.
Otherwise the surface would flicker because the disabled flag loads from the config.
*/
showIndicator: false,
isDisabled: false,
disabledUntil: 0,
// Since the disabled state is passed from the server, delay the
// render until we receive the exact disabled value. Otherwise,
// the indicator would flicker when it should be disabled.
isReady: false,
},
},
}
Expand All @@ -162,18 +153,6 @@ function getInitialState(
export function useErrorOverlayReducer(routerType: 'pages' | 'app') {
return useReducer((state: OverlayState, action: BusEvent): OverlayState => {
switch (action.type) {
case ACTION_STATIC_INDICATOR: {
return {
...state,
devToolsClientState: {
...state.devToolsClientState,
devIndicator: {
...state.devToolsClientState.devIndicator,
staticIndicator: action.staticIndicator,
},
},
}
}
case ACTION_BUILD_OK: {
return { ...state, buildError: null }
}
Expand Down Expand Up @@ -230,19 +209,32 @@ export function useErrorOverlayReducer(routerType: 'pages' | 'app') {
return state
}
}
case ACTION_DEV_TOOLS: {
case ACTION_STATIC_INDICATOR: {
return {
...state,
devToolsClientState: {
...state.devToolsClientState,
versionInfo: action.devTools.versionInfo,
debugInfo: action.devTools.debugInfo,
devIndicator: {
...state.devToolsClientState.devIndicator,
showIndicator: true,
disableDevIndicator:
shouldDisableDevIndicator ||
!!action.devTools.devIndicator.disabledUntil,
staticIndicator: action.staticIndicator,
},
},
}
}
case ACTION_DEV_TOOLS: {
return {
...state,
devToolsClientState: {
...action.devTools,
devIndicator: {
...action.devTools.devIndicator,
// Since the disabled state is passed from the server, delay the
// render until we receive the exact disabled value. Otherwise,
// the indicator would flicker when it should be disabled.
isReady: true,
// This is handled by ACTION_STATIC_INDICATOR action.
staticIndicator:
state.devToolsClientState.devIndicator.staticIndicator,
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ const state: OverlayState = {
debugInfo: { devtoolsFrontendUrl: undefined },
devIndicator: {
staticIndicator: true,
showIndicator: true,
disableDevIndicator: false,
isReady: true,
isDisabled: false,
disabledUntil: 0,
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function DevToolsIndicator({
setIsErrorOverlayOpen={setIsErrorOverlayOpen}
isTurbopack={!!process.env.TURBOPACK}
disabled={
state.devToolsClientState.devIndicator.disableDevIndicator ||
state.devToolsClientState.devIndicator.isDisabled ||
!isDevToolsIndicatorVisible
}
isBuildError={isBuildError}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ const state: OverlayState = {
debugInfo: { devtoolsFrontendUrl: undefined },
devIndicator: {
staticIndicator: true,
showIndicator: true,
disableDevIndicator: false,
isReady: true,
isDisabled: false,
disabledUntil: 0,
},
},
errors: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function DevOverlay({
const isBuildError = state.buildError !== null
return (
<>
{state.devToolsClientState.devIndicator.showIndicator && (
{state.devToolsClientState.devIndicator.isReady && (
<DevToolsIndicator
scale={scale}
setScale={setScale}
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/server/dev/dev-indicator-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export function getDisableDevIndicatorMiddleware() {
return middlewareResponse.methodNotAllowed(res)
}

devToolsServerState.devIndicator.isDisabled = true
devToolsServerState.devIndicator.disabledUntil =
Date.now() + COOLDOWN_TIME_MS

Expand Down
10 changes: 8 additions & 2 deletions packages/next/src/server/dev/dev-tools-server-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ import type { VersionInfo } from './parse-version-info'
import type { DebugInfo } from '../../client/components/react-dev-overlay/types'

export type DevToolsServerState = {
devIndicator: { disabledUntil: number }
devIndicator: {
isDisabled: boolean
disabledUntil: number
}
versionInfo: VersionInfo
debugInfo: DebugInfo
}

export const devToolsServerState: DevToolsServerState = {
devIndicator: { disabledUntil: 0 },
devIndicator: {
isDisabled: false,
disabledUntil: 0,
},
versionInfo: {
installed: '0.0.0',
staleness: 'unknown',
Expand Down
7 changes: 7 additions & 0 deletions packages/next/src/server/dev/hot-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,16 @@ export class WebpackHotMiddleware {
const middlewareStats = statsToJson(this.middlewareLatestStats?.stats)

if (devToolsServerState.devIndicator.disabledUntil < Date.now()) {
devToolsServerState.devIndicator.isDisabled = false
devToolsServerState.devIndicator.disabledUntil = 0
}

// __NEXT_DEV_INDICATOR is set to false when devIndicator is
// explicitly marked as false.
if (process.env.__NEXT_DEV_INDICATOR?.toString() === 'false') {
devToolsServerState.devIndicator.isDisabled = true
}

this.publish({
action: HMR_ACTIONS_SENT_TO_BROWSER.SYNC,
hash: stats.hash!,
Expand Down
7 changes: 7 additions & 0 deletions packages/next/src/server/dev/hot-reloader-turbopack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -840,9 +840,16 @@ export async function createHotReloaderTurbopack(
}

if (devToolsServerState.devIndicator.disabledUntil < Date.now()) {
devToolsServerState.devIndicator.isDisabled = false
devToolsServerState.devIndicator.disabledUntil = 0
}

// __NEXT_DEV_INDICATOR is set to false when devIndicator is
// explicitly marked as false.
if (process.env.__NEXT_DEV_INDICATOR?.toString() === 'false') {
devToolsServerState.devIndicator.isDisabled = true
}

;(async function () {
const versionInfo = await versionInfoPromise

Expand Down
Loading