Skip to content

Commit

Permalink
Fix hmr updates with rebuilding for build errors (#60676)
Browse files Browse the repository at this point in the history
### What

Sometimes the display of the error overlay is not consistent even when
you fixed the error

Example:
* You have a client page, and you add metadata export, Next.js will
error that it's not allowed in client component.
* You fix it, but the error overlay is not disappeared. Or you fix it,
then introduce it again, then fix it again, the error overlay is still
there.

### How

We're checking if the HMR webpack hash is changed to decide if we're
going to do a hot update in `BUILT` and `SYNC` event, but we update the
hash at the very beginning of the event handler. `isUpdateAvailable()`
will return `false` in the later calls but the hash has already changed
before. So we check if they change before applying hot updates, and then
use it later with `isUpdateAvailable()` to determine if necessary to
process a new hot update

Closes NEXT-2107

---------

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
  • Loading branch information
huozhi and timneutkens authored Jan 16, 2024
1 parent 521b0ad commit dc39448
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,16 @@ function handleAvailableHash(hash: string) {
mostRecentCompilationHash = hash
}

// Is there a newer version of this code available?
/**
* Is there a newer version of this code available?
* For webpack: Check if the hash changed compared to __webpack_hash__
* For Turbopack: Always true because it doesn't have __webpack_hash__
*/
function isUpdateAvailable() {
if (process.env.TURBOPACK) {
return true
}

/* globals __webpack_hash__ */
// __webpack_hash__ is the hash of the current compilation.
// It's a global variable injected by Webpack.
Expand Down Expand Up @@ -326,9 +334,6 @@ function processMessage(
})
)

// Compilation with warnings (e.g. ESLint).
const isHotUpdate = obj.action !== HMR_ACTIONS_SENT_TO_BROWSER.SYNC

// Print warnings to the console.
const formattedMessages = formatWebpackMessages({
warnings: warnings,
Expand All @@ -346,11 +351,7 @@ function processMessage(
console.warn(stripAnsi(formattedMessages.warnings[i]))
}

// Attempt to apply hot updates or reload.
if (isHotUpdate) {
handleHotUpdate(obj.updatedModules)
}
return
// No early return here as we need to apply modules in the same way between warnings only and compiles without warnings
}

sendMessage(
Expand All @@ -360,13 +361,8 @@ function processMessage(
})
)

const isHotUpdate =
obj.action !== HMR_ACTIONS_SENT_TO_BROWSER.SYNC &&
(!window.__NEXT_DATA__ || window.__NEXT_DATA__.page !== '/_error') &&
isUpdateAvailable()

// Attempt to apply hot updates or reload.
if (isHotUpdate) {
if (obj.action === HMR_ACTIONS_SENT_TO_BROWSER.BUILT) {
// Handle hot updates
handleHotUpdate(obj.updatedModules)
}
return
Expand Down
7 changes: 5 additions & 2 deletions test/development/acceptance-app/rsc-build-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ describe('Error overlay - RSC build errors', () => {
await cleanup()
})

// TODO: The error overlay is not closed when restoring the working code.
it.skip('should throw an error when metadata export is used in client components', async () => {
it('should throw an error when metadata export is used in client components', async () => {
const { session, cleanup } = await sandbox(
next,
undefined,
Expand Down Expand Up @@ -74,6 +73,10 @@ describe('Error overlay - RSC build errors', () => {
'You are attempting to export "generateMetadata" from a component marked with "use client", which is disallowed.'
)

// Fix the error again to test error overlay works with hmr rebuild
await session.patch(pageFile, content)
expect(await session.hasRedbox()).toBe(false)

await cleanup()
})

Expand Down

0 comments on commit dc39448

Please sign in to comment.