Skip to content
Merged
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
1 change: 1 addition & 0 deletions packages/next/src/build/templates/app-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ export async function handler(
subresourceIntegrityManifest,
serverActionsManifest,
clientReferenceManifest,
setCacheStatus: routerServerContext?.setCacheStatus,
setIsrStatus: routerServerContext?.setIsrStatus,
setReactDebugChannel: routerServerContext?.setReactDebugChannel,

Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/client/app-next-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ appBootstrap((assetPrefix) => {
try {
hydrate(instrumentationHooks, assetPrefix)
} finally {
renderAppDevOverlay(getOwnerStack, isRecoverableError)
const enableCacheIndicator = process.env.__NEXT_CACHE_COMPONENTS
renderAppDevOverlay(getOwnerStack, isRecoverableError, enableCacheIndicator)
}
})
7 changes: 6 additions & 1 deletion packages/next/src/client/app-next-turbopack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ appBootstrap((assetPrefix) => {
hydrate(instrumentationHooks, assetPrefix)
} finally {
if (process.env.NODE_ENV !== 'production') {
const enableCacheIndicator = process.env.__NEXT_CACHE_COMPONENTS
const { getOwnerStack } =
require('../next-devtools/userspace/app/errors/stitched-error') as typeof import('../next-devtools/userspace/app/errors/stitched-error')
const { renderAppDevOverlay } =
require('next/dist/compiled/next-devtools') as typeof import('next/dist/compiled/next-devtools')
renderAppDevOverlay(getOwnerStack, isRecoverableError)
renderAppDevOverlay(
getOwnerStack,
isRecoverableError,
enableCacheIndicator
)
}
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ export function processMessage(
sendMessage(JSON.stringify(response))
return
}
case HMR_MESSAGE_SENT_TO_BROWSER.CACHE_INDICATOR: {
dispatcher.onCacheIndicator(message.state)
return
}
case HMR_MESSAGE_SENT_TO_BROWSER.MIDDLEWARE_CHANGES:
case HMR_MESSAGE_SENT_TO_BROWSER.CLIENT_CHANGES:
case HMR_MESSAGE_SENT_TO_BROWSER.SERVER_ONLY_CHANGES:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ function processMessage(message: HmrMessageSentToBrowser) {
case HMR_MESSAGE_SENT_TO_BROWSER.DEVTOOLS_CONFIG:
dispatcher.onDevToolsConfig(message.data)
break
case HMR_MESSAGE_SENT_TO_BROWSER.CACHE_INDICATOR:
case HMR_MESSAGE_SENT_TO_BROWSER.REACT_DEBUG_CHUNK:
// Only relevant for app router.
break
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/client/page-bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export function pageBootstrap(assetPrefix: string) {
case HMR_MESSAGE_SENT_TO_BROWSER.REACT_DEBUG_CHUNK:
case HMR_MESSAGE_SENT_TO_BROWSER.REQUEST_CURRENT_ERROR_STATE:
case HMR_MESSAGE_SENT_TO_BROWSER.REQUEST_PAGE_METADATA:
case HMR_MESSAGE_SENT_TO_BROWSER.CACHE_INDICATOR:
// Most of these action types are handled in
// src/client/dev/hot-reloader/pages/hot-reloader-pages.ts and
// src/client/dev/hot-reloader/app/hot-reloader-app.tsx
Expand Down
19 changes: 17 additions & 2 deletions packages/next/src/next-devtools/dev-overlay.browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
ACTION_DEVTOOLS_CONFIG,
type OverlayState,
type DispatcherEvent,
ACTION_CACHE_INDICATOR,
} from './dev-overlay/shared'

import {
Expand All @@ -33,6 +34,7 @@ import {
type ActionDispatch,
} from 'react'
import { createRoot } from 'react-dom/client'
import type { CacheIndicatorState } from './dev-overlay/cache-indicator'
import { FontStyles } from './dev-overlay/font/font-styles'
import type { HydrationErrorState } from './shared/hydration-error'
import type { DebugInfo } from './shared/types'
Expand All @@ -55,6 +57,7 @@ export interface Dispatcher {
onDebugInfo(debugInfo: DebugInfo): void
onBeforeRefresh(): void
onRefresh(): void
onCacheIndicator(status: CacheIndicatorState): void
onStaticIndicator(status: 'pending' | 'static' | 'dynamic' | 'disabled'): void
onDevIndicator(devIndicator: DevIndicatorServerState): void
onDevToolsConfig(config: DevToolsConfig): void
Expand Down Expand Up @@ -147,6 +150,11 @@ export const dispatcher: Dispatcher = {
dispatch({ type: ACTION_VERSION_INFO, versionInfo })
}
),
onCacheIndicator: createQueuable(
(dispatch: Dispatch, status: CacheIndicatorState) => {
dispatch({ type: ACTION_CACHE_INDICATOR, cacheIndicator: status })
}
),
onStaticIndicator: createQueuable(
(
dispatch: Dispatch,
Expand Down Expand Up @@ -230,12 +238,14 @@ function replayQueuedEvents(dispatch: NonNullable<typeof maybeDispatch>) {
}

function DevOverlayRoot({
enableCacheIndicator,
getOwnerStack,
getSquashedHydrationErrorDetails,
isRecoverableError,
routerType,
shadowRoot,
}: {
enableCacheIndicator: boolean
getOwnerStack: (error: Error) => string | null | undefined
getSquashedHydrationErrorDetails: (error: Error) => HydrationErrorState | null
isRecoverableError: (error: Error) => boolean
Expand All @@ -245,7 +255,8 @@ function DevOverlayRoot({
const [state, dispatch] = useErrorOverlayReducer(
routerType,
getOwnerStack,
isRecoverableError
isRecoverableError,
enableCacheIndicator
)

useEffect(() => {
Expand Down Expand Up @@ -319,7 +330,8 @@ function getSquashedHydrationErrorDetailsApp() {

export function renderAppDevOverlay(
getOwnerStack: (error: Error) => string | null | undefined,
isRecoverableError: (error: Error) => boolean
isRecoverableError: (error: Error) => boolean,
enableCacheIndicator: boolean
): void {
if (isPagesMounted) {
// Switching between App and Pages Router is always a hard navigation
Expand Down Expand Up @@ -361,6 +373,7 @@ export function renderAppDevOverlay(
// At least it won't unmount any user code if it errors.
root.render(
<DevOverlayRoot
enableCacheIndicator={enableCacheIndicator}
getOwnerStack={getOwnerStack}
getSquashedHydrationErrorDetails={getSquashedHydrationErrorDetailsApp}
isRecoverableError={isRecoverableError}
Expand Down Expand Up @@ -426,6 +439,8 @@ export function renderPagesDevOverlay(
// At least it won't unmount any user code if it errors.
root.render(
<DevOverlayRoot
// Pages Router does not support Cache Components
enableCacheIndicator={false}
getOwnerStack={getOwnerStack}
getSquashedHydrationErrorDetails={getSquashedHydrationErrorDetails}
isRecoverableError={isRecoverableError}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export type ServerCacheStatus = 'filling' | 'filled'
export type CacheIndicatorState = 'disabled' | 'ready' | ServerCacheStatus
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ export function DevToolsIndicator() {
<Toast
id="devtools-indicator"
data-nextjs-toast
// TODO: Remove once we have actual UI for this.
data-nextjs-cache-indicator={
state.cacheIndicator === 'disabled' ? undefined : state.cacheIndicator
}
style={
{
'--animate-out-duration-ms': `${MENU_DURATION_MS}ms`,
Expand Down
22 changes: 19 additions & 3 deletions packages/next/src/next-devtools/dev-overlay/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { DebugInfo } from '../shared/types'
import type { DevIndicatorServerState } from '../../server/dev/dev-indicator-server-state'
import { parseStack } from '../../server/lib/parse-stack'
import { isConsoleError } from '../shared/console-error'
import type { CacheIndicatorState } from './cache-indicator'

export type DevToolsConfig = {
theme?: 'light' | 'dark' | 'system'
Expand Down Expand Up @@ -49,6 +50,7 @@ export interface OverlayState {
readonly notFound: boolean
readonly buildingIndicator: boolean
readonly renderingIndicator: boolean
readonly cacheIndicator: CacheIndicatorState
readonly staticIndicator: 'pending' | 'static' | 'dynamic' | 'disabled'
readonly showIndicator: boolean
readonly disableDevIndicator: boolean
Expand All @@ -72,6 +74,7 @@ export interface OverlayState {
type DevtoolsPanelName = string
export type OverlayDispatch = React.Dispatch<DispatcherEvent>

export const ACTION_CACHE_INDICATOR = 'cache-indicator'
export const ACTION_STATIC_INDICATOR = 'static-indicator'
export const ACTION_BUILD_OK = 'build-ok'
export const ACTION_BUILD_ERROR = 'build-error'
Expand Down Expand Up @@ -110,6 +113,11 @@ export const STORE_KEY_SHARED_PANEL_LOCATION =
export const ACTION_DEVTOOL_UPDATE_ROUTE_STATE =
'segment-explorer-update-route-state'

interface CacheIndicatorAction {
type: typeof ACTION_CACHE_INDICATOR
cacheIndicator: CacheIndicatorState
}

interface StaticIndicatorAction {
type: typeof ACTION_STATIC_INDICATOR
staticIndicator: 'pending' | 'static' | 'dynamic' | 'disabled'
Expand Down Expand Up @@ -216,6 +224,7 @@ export type DispatcherEvent =
| UnhandledErrorAction
| UnhandledRejectionAction
| VersionInfoAction
| CacheIndicatorAction
| StaticIndicatorAction
| DebugInfoAction
| DevIndicatorAction
Expand Down Expand Up @@ -263,6 +272,7 @@ export const INITIAL_OVERLAY_STATE: Omit<
errors: [],
notFound: false,
renderingIndicator: false,
cacheIndicator: 'disabled',
staticIndicator: 'disabled',
/*
This is set to `true` when we can reliably know
Expand All @@ -287,7 +297,8 @@ export const INITIAL_OVERLAY_STATE: Omit<
}

function getInitialState(
routerType: 'pages' | 'app'
routerType: 'pages' | 'app',
enableCacheIndicator: boolean
): OverlayState & { routerType: 'pages' | 'app' } {
return {
...INITIAL_OVERLAY_STATE,
Expand All @@ -296,13 +307,15 @@ function getInitialState(
// TODO: Should be the same default as App Router once we surface console.error in Pages Router.
isErrorOverlayOpen: routerType === 'pages',
routerType,
cacheIndicator: enableCacheIndicator ? 'ready' : 'disabled',
}
}

export function useErrorOverlayReducer(
routerType: 'pages' | 'app',
getOwnerStack: (error: Error) => string | null | undefined,
isRecoverableError: (error: Error) => boolean
isRecoverableError: (error: Error) => boolean,
enableCacheIndicator: boolean
) {
function pushErrorFilterDuplicates(
events: readonly SupportedErrorEvent[],
Expand Down Expand Up @@ -349,6 +362,9 @@ export function useErrorOverlayReducer(
case ACTION_DEBUG_INFO: {
return { ...state, debugInfo: action.debugInfo }
}
case ACTION_CACHE_INDICATOR: {
return { ...state, cacheIndicator: action.cacheIndicator }
}
case ACTION_STATIC_INDICATOR: {
return { ...state, staticIndicator: action.staticIndicator }
}
Expand Down Expand Up @@ -496,6 +512,6 @@ export function useErrorOverlayReducer(
}
}
},
getInitialState(routerType)
getInitialState(routerType, enableCacheIndicator)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
ACTION_BUILD_OK,
ACTION_BUILDING_INDICATOR_HIDE,
ACTION_BUILDING_INDICATOR_SHOW,
ACTION_CACHE_INDICATOR,
ACTION_DEBUG_INFO,
ACTION_DEV_INDICATOR,
ACTION_DEV_INDICATOR_SET,
Expand Down Expand Up @@ -81,6 +82,7 @@ export function useStorybookOverlayReducer(initialState?: OverlayState) {
case ACTION_REFRESH:
case ACTION_RENDERING_INDICATOR_HIDE:
case ACTION_RENDERING_INDICATOR_SHOW:
case ACTION_CACHE_INDICATOR:
case ACTION_STATIC_INDICATOR:
case ACTION_UNHANDLED_ERROR:
case ACTION_UNHANDLED_REJECTION:
Expand Down
20 changes: 17 additions & 3 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2768,9 +2768,13 @@ async function renderWithRestartOnCacheMissInDev(
getPayload: (requestStore: RequestStore) => Promise<RSCPayload>,
onError: (error: unknown) => void
) {
const { renderOpts } = ctx
const { clientReferenceManifest, ComponentMod, setReactDebugChannel } =
renderOpts
const { htmlRequestId, renderOpts, requestId } = ctx
const {
clientReferenceManifest,
ComponentMod,
setCacheStatus,
setReactDebugChannel,
} = renderOpts
assertClientReferenceManifest(clientReferenceManifest)

// If the render is restarted, we'll recreate a fresh request store
Expand Down Expand Up @@ -2898,6 +2902,10 @@ async function renderWithRestartOnCacheMissInDev(
}
}

if (process.env.NODE_ENV === 'development') {
setCacheStatus!('filling', htmlRequestId, requestId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsafe non-null assertions on setCacheStatus could cause a runtime error. The function is optional and can be undefined even when NODE_ENV === 'development', but the code uses non-null assertions without checking if the function is actually defined.

View Details
📝 Patch Details
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx
index bc03f4e2fa..a28e3d2a74 100644
--- a/packages/next/src/server/app-render/app-render.tsx
+++ b/packages/next/src/server/app-render/app-render.tsx
@@ -2804,8 +2804,8 @@ async function renderWithRestartOnCacheMissInDev(
     }
   }
 
-  if (process.env.NODE_ENV === 'development') {
-    setCacheStatus!('filling', htmlRequestId, requestId)
+  if (process.env.NODE_ENV === 'development' && setCacheStatus) {
+    setCacheStatus('filling', htmlRequestId, requestId)
   }
 
   // Cache miss. We will use the initial render to fill caches, and discard its result.
@@ -2863,10 +2863,10 @@ async function renderWithRestartOnCacheMissInDev(
     }
   )
 
-  if (process.env.NODE_ENV === 'development') {
+  if (process.env.NODE_ENV === 'development' && setCacheStatus) {
     // TODO: Don't know if this is the right time.
     // It's not wired up on the frontend though.
-    setCacheStatus!('filled', htmlRequestId, requestId)
+    setCacheStatus('filled', htmlRequestId, requestId)
   }
 
   return {

Analysis

Unsafe non-null assertions on setCacheStatus in cache component rendering

What fails: Lines 2808 and 2869 in packages/next/src/server/app-render/app-render.tsx use non-null assertions (setCacheStatus!()) to call an optional function that can be undefined, causing a runtime TypeError if the function is not available.

How to reproduce:

  1. Configure Next.js with experimental.cacheComponents: true in next.config.js
  2. Set process.env.NODE_ENV = 'development' externally (before Next.js initialization)
  3. Start the server with dev mode disabled (passing opts.dev = false)
  4. Make a request to trigger the renderWithRestartOnCacheMissInDev code path

Result: Runtime error:

TypeError: setCacheStatus is not a function

Expected: The code should safely check if the function exists before calling it.

Root cause: The function setCacheStatus is defined as an optional property in the RenderOptsPartial type. It gets assigned a value only if both:

  • config.experimental.cacheComponents is true AND
  • devBundlerService is defined (which requires opts.dev to be true)

However, the guard that calls this function only checks process.env.NODE_ENV === 'development' && experimental.cacheComponents. Since NODE_ENV can be set externally before Next.js initializes, it's possible for the guard to pass even when opts.dev is false, making devBundlerService undefined and setCacheStatus unassigned.

Fix: Added null checks before calling setCacheStatus at both locations to ensure the function is only invoked if it's actually defined:

  • Line 2807: Changed if (process.env.NODE_ENV === 'development') to if (process.env.NODE_ENV === 'development' && setCacheStatus)
  • Line 2866: Changed if (process.env.NODE_ENV === 'development') to if (process.env.NODE_ENV === 'development' && setCacheStatus)
  • Removed the non-null assertion operators (!) since we now have proper guards

Copy link
Member Author

Choose a reason for hiding this comment

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

This branch should only be hit with cache components. If not, that'd be a bug.

}

// Cache miss. We will use the initial render to fill caches, and discard its result.
// Then, we can render again with warm caches.

Expand Down Expand Up @@ -2967,6 +2975,12 @@ async function renderWithRestartOnCacheMissInDev(
)
)

if (process.env.NODE_ENV === 'development') {
// TODO: Don't know if this is the right time.
// It's not wired up on the frontend though.
Comment on lines +2979 to +2980
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine here. Technically it could be directly after awaiting cacheReady, but then we'd have a bit more nothing-time before the first chunks stream in. So I think it's better to prolong the filling state until here.

setCacheStatus!('filled', htmlRequestId, requestId)
}

return {
stream: finalServerStream,
debugChannel,
Expand Down
6 changes: 6 additions & 0 deletions packages/next/src/server/app-render/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type { NextRequestHint } from '../web/adapter'
import type { BaseNextRequest } from '../base-http'
import type { IncomingMessage } from 'http'
import type { RenderResumeDataCache } from '../resume-data-cache/resume-data-cache'
import type { ServerCacheStatus } from '../../next-devtools/dev-overlay/cache-indicator'

const dynamicParamTypesSchema = s.enums(['c', 'ci', 'oc', 'd', 'di'])

Expand Down Expand Up @@ -96,6 +97,11 @@ export interface RenderOptsPartial {
}
isOnDemandRevalidate?: boolean
isPossibleServerAction?: boolean
setCacheStatus?: (
status: ServerCacheStatus,
htmlRequestId: string,
requestId: string
) => void
setIsrStatus?: (key: string, value: boolean | undefined) => void
setReactDebugChannel?: (
debugChannel: { readable: ReadableStream<Uint8Array> },
Expand Down
Loading
Loading