Skip to content
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.

Sign up for GitHub

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

chore(HMR clients): Clean up tryApplyUpdates, reduce differences between app/pages versions #77219

Merged
merged 1 commit into from
Mar 19, 2025
Merged

chore(HMR clients): Clean up tryApplyUpdates, reduce differences between app/pages versions #77219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 37 additions & 70 deletions packages/next/src/client/components/react-dev-overlay/app/hot-reloader-client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,6 @@ export function waitForWebpackRuntimeHotUpdate() {
return pendingHotUpdateWebpack
}

function handleSuccessfulHotUpdateWebpack(
Copy link
Member Author

Choose a reason for hiding this comment

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

I inlined this, it only had one callsite.

dispatcher: Dispatcher,
sendMessage: (message: string) => void,
updatedModules: ReadonlyArray<string>
) {
resolvePendingHotUpdateWebpack()
dispatcher.onBuildOk()
reportHmrLatency(
sendMessage,
updatedModules,
webpackStartMsSinceEpoch!,
Date.now()
)

dispatcher.onRefresh()
}

// There is a newer version of the code available.
function handleAvailableHash(hash: string) {
// Update last known compilation hash.
Expand Down Expand Up @@ -160,10 +143,8 @@ function performFullReload(err: any, sendMessage: any) {
}

// Attempt to update code on the fly, fall back to a hard reload.
function tryApplyUpdates(
onBeforeUpdate: (hasUpdates: boolean) => void,
onHotUpdateSuccess: (updatedModules: string[]) => void,
sendMessage: any,
function tryApplyUpdatesWebpack(
sendMessage: (message: string) => void,
dispatcher: Dispatcher
) {
if (!isUpdateAvailable() || !canApplyUpdates()) {
Expand All @@ -173,8 +154,11 @@ function tryApplyUpdates(
return
}

function handleApplyUpdates(err: any, updatedModules: string[] | null) {
if (err || RuntimeErrorHandler.hadRuntimeError || !updatedModules) {
function handleApplyUpdates(
err: any,
updatedModules: (string | number)[] | null
Copy link
Member Author

Choose a reason for hiding this comment

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

The type of string[] was incorrect, it's actually (string | number)[]:

https://github.com/webpack/webpack/blob/3aa6b6bc3a6/module.d.ts#L139-L142

) {
if (err || RuntimeErrorHandler.hadRuntimeError || updatedModules == null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Using updatedModules == null here because it's more explicit. Empty arrays are truthy (so there's no change in behavior on this line), but that can be confusing.

if (err) {
console.warn(REACT_REFRESH_FULL_RELOAD)
} else if (RuntimeErrorHandler.hadRuntimeError) {
Expand All @@ -184,50 +168,50 @@ function tryApplyUpdates(
return
}

const hasUpdates = Boolean(updatedModules.length)
if (typeof onHotUpdateSuccess === 'function') {
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 was always taken, based on the type signature.

// Maybe we want to do something.
onHotUpdateSuccess(updatedModules)
}
dispatcher.onBuildOk()
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 is the start of the inlined version of onHotUpdateSuccess


if (isUpdateAvailable()) {
// While we were updating, there was a new update! Do it again.
tryApplyUpdates(
hasUpdates ? () => {} : onBeforeUpdate,
hasUpdates ? () => dispatcher.onBuildOk() : onHotUpdateSuccess,
sendMessage,
dispatcher
)
} else {
dispatcher.onBuildOk()
Copy link
Member Author

Choose a reason for hiding this comment

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

We were calling this twice before. That became obvious when inlining onHotUpdateSuccess/handleSuccessfulHotUpdateWebpack.

if (process.env.__NEXT_TEST_MODE) {
afterApplyUpdates(() => {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB()
self.__NEXT_HMR_CB = null
}
})
}
tryApplyUpdatesWebpack(sendMessage, dispatcher)
Comment on lines 174 to +175
Copy link
Member Author

Choose a reason for hiding this comment

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

Slight difference here...

Old version: Run onRefresh immediately (as part of onHotUpdateSuccess/handleSuccessfulHotUpdateWebpack), and then avoiding calling it again later by passing around a different version of onHotUpdateSuccess.

New version: Don't call onRefresh until there's no more updates to apply.

onRefresh is used to control what happens if an uncaught exception or rejection happens during HMR.

return
}

dispatcher.onRefresh()
resolvePendingHotUpdateWebpack()
reportHmrLatency(
sendMessage,
updatedModules,
webpackStartMsSinceEpoch!,
Date.now()
)

if (process.env.__NEXT_TEST_MODE) {
afterApplyUpdates(() => {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB()
self.__NEXT_HMR_CB = null
}
})
}
}

// https://webpack.js.org/api/hot-module-replacement/#check
module.hot
.check(/* autoApply */ false)
.then((updatedModules: any[] | null) => {
if (!updatedModules) {
.then((updatedModules: (string | number)[] | null) => {
if (updatedModules == null) {
return null
}

if (typeof onBeforeUpdate === 'function') {
const hasUpdates = Boolean(updatedModules.length)
onBeforeUpdate(hasUpdates)
}
// We should always handle an update, even if updatedModules is empty (but
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if there are actually cases where updatedModules would be non-null and empty.

// non-null) for any reason. That's what webpack would normally do:
// https://github.com/webpack/webpack/blob/3aa6b6bc3a64/lib/hmr/HotModuleReplacement.runtime.js#L296-L298
dispatcher.onBeforeRefresh()
// https://webpack.js.org/api/hot-module-replacement/#apply
return module.hot.apply()
})
.then(
(updatedModules: any[] | null) => {
(updatedModules: (string | number)[] | null) => {
handleApplyUpdates(null, updatedModules)
},
(err: any) => {
Expand All @@ -236,7 +220,7 @@ function tryApplyUpdates(
)
}

/** Handles messages from the sevrer for the App Router. */
/** Handles messages from the server for the App Router. */
function processMessage(
obj: HMR_ACTION_TYPES,
sendMessage: (message: string) => void,
Expand Down Expand Up @@ -288,24 +272,7 @@ function processMessage(
}
dispatcher.onBuildOk()
} else {
tryApplyUpdates(
function onBeforeHotUpdate(hasUpdates: boolean) {
if (hasUpdates) {
dispatcher.onBeforeRefresh()
}
},
function onSuccessfulHotUpdate(webpackUpdatedModules: string[]) {
// Only dismiss it when we're sure it's a hot update.
// Otherwise it would flicker right before the reload.
handleSuccessfulHotUpdateWebpack(
dispatcher,
sendMessage,
webpackUpdatedModules
)
},
sendMessage,
dispatcher
)
tryApplyUpdatesWebpack(sendMessage, dispatcher)
}
}

Expand Down
107 changes: 43 additions & 64 deletions packages/next/src/client/components/react-dev-overlay/pages/hot-reloader-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
* SOFTWARE.
*/

/// <reference types="webpack/module.d.ts" />

// This file is a modified version of the Create React App HMR dev client that
// can be found here:
// https://github.com/facebook/create-react-app/blob/v3.4.1/packages/react-dev-utils/webpackHotDevClient.js
Expand Down Expand Up @@ -55,10 +57,8 @@ import {
} from '../shared'
import { RuntimeErrorHandler } from '../../errors/runtime-error-handler'
import reportHmrLatency from '../utils/report-hmr-latency'
import {
extractModulesFromTurbopackMessage,
TurbopackHmr,
} from '../utils/turbopack-hot-reloader-common'
import { TurbopackHmr } from '../utils/turbopack-hot-reloader-common'

// This alternative WebpackDevServer combines the functionality of:
// https://github.com/webpack/webpack-dev-server/blob/webpack-1/client/index.js
// https://github.com/webpack/webpack/blob/webpack-1/hot/dev-server.js
Expand Down Expand Up @@ -149,7 +149,7 @@ function handleSuccess() {

// Attempt to apply hot updates or reload.
if (isHotUpdate) {
tryApplyUpdates(onBeforeFastRefresh, onFastRefresh)
tryApplyUpdatesWebpack()
}
}

Expand Down Expand Up @@ -189,7 +189,7 @@ function handleWarnings(warnings: any) {

// Attempt to apply hot updates or reload.
if (isHotUpdate) {
tryApplyUpdates(onBeforeFastRefresh, onFastRefresh)
tryApplyUpdatesWebpack()
}
}

Expand Down Expand Up @@ -233,30 +233,6 @@ const turbopackHmr: TurbopackHmr | null = process.env.TURBOPACK
: null
let isrManifest: Record<string, boolean> = {}

function onBeforeFastRefresh(updatedModules: string[]) {
if (updatedModules.length > 0) {
// Only trigger a pending state if we have updates to apply
// (cf. onFastRefresh)
onBeforeRefresh()
}
}

function onFastRefresh(updatedModules: ReadonlyArray<string> = []) {
onBuildOk()
if (updatedModules.length === 0) {
return
}

onRefresh()

reportHmrLatency(
sendMessage,
updatedModules,
webpackStartMsSinceEpoch!,
Date.now()
)
}

// There is a newer version of the code available.
function handleAvailableHash(hash: string) {
// Update last known compilation hash.
Expand All @@ -282,7 +258,7 @@ export function handleStaticIndicator() {
}
}

/** Handles messages from the sevrer for the Pages Router. */
/** Handles messages from the server for the Pages Router. */
function processMessage(obj: HMR_ACTION_TYPES) {
if (!('action' in obj)) {
return
Expand Down Expand Up @@ -325,6 +301,7 @@ function processMessage(obj: HMR_ACTION_TYPES) {
return handleErrors(errors)
}

// NOTE: Turbopack does not currently send warnings
const hasWarnings = Boolean(warnings && warnings.length)
if (hasWarnings) {
sendMessage(
Expand Down Expand Up @@ -371,8 +348,7 @@ function processMessage(obj: HMR_ACTION_TYPES) {
break
}
case HMR_ACTIONS_SENT_TO_BROWSER.TURBOPACK_MESSAGE: {
const updatedModules = extractModulesFromTurbopackMessage(obj.data)
onBeforeFastRefresh([...updatedModules])
onBeforeRefresh()
for (const listener of turbopackMessageListeners) {
listener({
type: HMR_ACTIONS_SENT_TO_BROWSER.TURBOPACK_MESSAGE,
Expand Down Expand Up @@ -424,10 +400,7 @@ function afterApplyUpdates(fn: () => void) {
}

// Attempt to update code on the fly, fall back to a hard reload.
function tryApplyUpdates(
onBeforeHotUpdate: ((updatedModules: string[]) => unknown) | undefined,
onHotUpdateSuccess: (updatedModules: string[]) => unknown
) {
function tryApplyUpdatesWebpack() {
if (!module.hot) {
// HotModuleReplacementPlugin is not in Webpack configuration.
console.error('HotModuleReplacementPlugin is not in Webpack configuration.')
Expand All @@ -440,8 +413,11 @@ function tryApplyUpdates(
return
}

function handleApplyUpdates(err: any, updatedModules: string[] | null) {
if (err || RuntimeErrorHandler.hadRuntimeError || !updatedModules) {
function handleApplyUpdates(
err: any,
updatedModules: (string | number)[] | null
) {
if (err || RuntimeErrorHandler.hadRuntimeError || updatedModules == null) {
if (err) {
console.warn(REACT_REFRESH_FULL_RELOAD)
} else if (RuntimeErrorHandler.hadRuntimeError) {
Expand All @@ -451,46 +427,49 @@ function tryApplyUpdates(
return
}

if (typeof onHotUpdateSuccess === 'function') {
// Maybe we want to do something.
onHotUpdateSuccess(updatedModules)
}
onBuildOk()

if (isUpdateAvailable()) {
// While we were updating, there was a new update! Do it again.
// However, this time, don't trigger a pending refresh state.
tryApplyUpdates(
updatedModules.length > 0 ? undefined : onBeforeHotUpdate,
updatedModules.length > 0 ? onBuildOk : onHotUpdateSuccess
)
} else {
onBuildOk()
if (process.env.__NEXT_TEST_MODE) {
afterApplyUpdates(() => {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB()
self.__NEXT_HMR_CB = null
}
})
}
tryApplyUpdatesWebpack()
return
}

onRefresh()
reportHmrLatency(
sendMessage,
updatedModules,
webpackStartMsSinceEpoch!,
Date.now()
)

if (process.env.__NEXT_TEST_MODE) {
afterApplyUpdates(() => {
if (self.__NEXT_HMR_CB) {
self.__NEXT_HMR_CB()
self.__NEXT_HMR_CB = null
}
})
}
}

// https://webpack.js.org/api/hot-module-replacement/#check
module.hot
.check(/* autoApply */ false)
.then((updatedModules: any) => {
if (!updatedModules) {
.then((updatedModules: (string | number)[] | null) => {
if (updatedModules == null) {
return null
}

if (typeof onBeforeHotUpdate === 'function') {
onBeforeHotUpdate(updatedModules)
}
// We should always handle an update, even if updatedModules is empty (but
// non-null) for any reason. That's what webpack would normally do:
// https://github.com/webpack/webpack/blob/3aa6b6bc3a64/lib/hmr/HotModuleReplacement.runtime.js#L296-L298
onBeforeRefresh()
// https://webpack.js.org/api/hot-module-replacement/#apply
return module.hot.apply()
})
.then(
(updatedModules: any) => {
(updatedModules: (string | number)[] | null) => {
handleApplyUpdates(null, updatedModules)
},
(err: any) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/client/components/react-dev-overlay/utils/report-hmr-latency.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ declare global {

export default function reportHmrLatency(
sendMessage: (message: string) => void,
updatedModules: ReadonlyArray<string>,
updatedModules: ReadonlyArray<string | number>,
startMsSinceEpoch: number,
endMsSinceEpoch: number
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/client/components/react-dev-overlay/utils/turbopack-hot-reloader-common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class TurbopackHmr {
}
}

export function extractModulesFromTurbopackMessage(
function extractModulesFromTurbopackMessage(
data: TurbopackUpdate | TurbopackUpdate[]
): Set<string> {
const updatedModules: Set<string> = new Set()
Expand Down
Loading