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

Jump to bottom

Fix "apply() is only allowed in ready status (state: idle)" HMR errors #43242

Merged
merged 2 commits into from Nov 27, 2022
Merged

Fix "apply() is only allowed in ready status (state: idle)" HMR errors #43242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da9aa10
Do not apply if status is idle
colinking Nov 22, 2022
576e432
Merge branch 'canary' into fix-apply-when-idle
ijjk Nov 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx
View file
Open in desktop
Expand Up @@ -144,7 +144,7 @@ function tryApplyUpdates(
return
}

const hasUpdates = Boolean(updatedModules?.length)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed these optional chaining checks where there is already a null check on updatedModules.

const hasUpdates = Boolean(updatedModules.length)
if (typeof onHotUpdateSuccess === 'function') {
// Maybe we want to do something.
onHotUpdateSuccess(hasUpdates)
Expand Down Expand Up @@ -176,16 +176,20 @@ function tryApplyUpdates(
module.hot
.check(/* autoApply */ false)
.then((updatedModules: any[] | null) => {
const hasUpdates = Boolean(updatedModules?.length)
if (!updatedModules) {
return null
}

if (typeof onBeforeUpdate === 'function') {
const hasUpdates = Boolean(updatedModules.length)
onBeforeUpdate(hasUpdates)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, hasUpdates may not be necessary. Seems confusing to handle the null and empty array cases differently. I don't have enough webpack knowledge to know if we should still call .apply() in the latter case, but that does appear to be what webpack does when autoApply=true.

}
// https://webpack.js.org/api/hot-module-replacement/#apply
// @ts-expect-error module.hot exists
return module.hot.apply()
})
.then(
(updatedModules: any) => {
(updatedModules: any[] | null) => {
handleApplyUpdates(null, updatedModules)
},
(err: any) => {
Expand Down
8 changes: 6 additions & 2 deletions packages/next/client/dev/error-overlay/hot-dev-client.js
View file
Open in desktop
Expand Up @@ -337,7 +337,7 @@ function tryApplyUpdates(onBeforeHotUpdate, onHotUpdateSuccess) {
return
}

const hasUpdates = Boolean(updatedModules?.length)
const hasUpdates = Boolean(updatedModules.length)
if (typeof onHotUpdateSuccess === 'function') {
// Maybe we want to do something.
onHotUpdateSuccess(hasUpdates)
Expand Down Expand Up @@ -367,8 +367,12 @@ function tryApplyUpdates(onBeforeHotUpdate, onHotUpdateSuccess) {
module.hot
.check(/* autoApply */ false)
.then((updatedModules) => {
if (!updatedModules) {
return null
}

if (typeof onBeforeHotUpdate === 'function') {
const hasUpdates = Boolean(updatedModules?.length)
const hasUpdates = Boolean(updatedModules.length)
onBeforeHotUpdate(hasUpdates)
}
return module.hot.apply()
Expand Down