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.

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

webpackHotDevClient no longer reloads page on HMR update error since fastcheck support was added #11161

Closed
Sidnioulz opened this issue Jun 30, 2021 · 3 comments

Comments

@Sidnioulz
Copy link

Sidnioulz commented Jun 30, 2021

Describe the bug

Please kindly note that this bug is not a duplicate of #10078.

Bug description

Prior to the addition of experimental support for fast-refresh (#8582), webpackHotDevClient would resort to reloading the page on HMR updates if it could not identify the updated module, or if an error occurred.

Such a behaviour was useful. Those situations occur frequently for people who eject and add Webpack plugins, eg. an asset not in mentioned in the JS entries was re-emitted, therefore not changing the compilation hash; or a loader writes an incorrect update JSON for an asset they emit (html-webpack-copy-plugin, in my case, is sending incorrect updates).

It's also possible for update errors to occur due to user code on non-ejected projects. We have a (proprietary) project where some JS files do not have a module.hot.accept handler and generate HMR errors when updated. In those cases, we're happy to resort to reloading the page.

Root cause

Now, the following commit: c5b96c2#diff-fcdb901a67f29b1a0b22c42dbf36bb167c5262e2994675bd4d2650e8c4dd6c8aL246 introduced support for React Fast Refresh by disabling the force-reload if Fast Refresh is on.

    function handleApplyUpdates(err, updatedModules) {
    const hasReactRefresh = process.env.FAST_REFRESH;
    const wantsForcedReload = err || !updatedModules || hadRuntimeError;
    // React refresh can handle hot-reloading over errors.
    if (!hasReactRefresh && wantsForcedReload) {
      window.location.reload();
      return;
    }

This might be legitimate in the case where no updates were identified (!updatedModules), or when there was a prior runtime error that could be fixed by the update (hadRuntimeError) but I believe if an error was reported by Webpack's bootstrap script, Fast Refresh shouldn't be expected to work since the update information is reportedly incorrect.

I believe the following code would be more appropriate behaviour:

    function handleApplyUpdates(webpackErr, updatedModules) {
    const hasReactRefresh = process.env.FAST_REFRESH;
    const wantsForcedReload = !updatedModules || hadRuntimeError;
    // React refresh can handle hot-reloading over runtime errors.
    if (webpackErr || (!hasReactRefresh && wantsForcedReload)) {
      window.location.reload();
      return;
    }

Did you try recovering your dependencies?

  • Dependencies up to date.
  • yarn --version 1.22.10

Which terms did you search for in User Guide?

ø

Environment

Environment Info:

  current version of create-react-app: 3.4.1
  running from /home/steve/.config/yarn/global/node_modules/create-react-app

  System:
    OS: Linux 5.12 Arch Linux
    CPU: (12) x64 Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
  Binaries:
    Node: 16.3.0 - /usr/bin/node
    Yarn: 1.22.10 - /usr/bin/yarn
    npm: 7.16.0 - ~/Development/diplp-front-web/node_modules/.bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: 89.0
  npmPackages:
    react: ^17.0.2 => 17.0.2 (16.14.0)
    react-dom: ^17.0.2 => 17.0.2 
    react-scripts:  4.0.1 
  npmGlobalPackages:
    create-react-app: Not Found

Steps to reproduce

(Write your steps here:)

  1. Cause a HMR error (you can write a local file override of your webpack/bootstrap.js in your devTools and make the request.onreadystatechange handler always reject; sadly I can't share a MWE with systematic error behaviour at the moment)
  2. Notice that the page does not refresh when a source file is updated

Expected behavior

The page should reload when a HMR error occurs, so that the update is applied nevertheless.

Actual behavior

The page does not automatically reload.

Reproducible demo

Sadly, my projects are my company's property and I'm unable to share them. There is significant boilerplate going into the projects where I can reproduce the bug, and I would need hours to extract a MWE and re-link all this code together in order to showcase the issue.

Errors can be somewhat easily simulated with a devtools override of http://localhost:3000/<projectAbsolutePathOnDisk>/webpack/bootstrap.js.

  1. Open Devtools
  2. Go to Source tab
  3. In the leftside panel, next to pages, click on the double caret >>, and on the "Overrides" menu item
  4. Enable local overrides for the folder where the CRA project resides
  5. Press Ctrl+P and type "webpackbootstrap"
  6. Select the bootstrap matching the path of the CRA project if multiple entries are found
  7. In equest.onreadystatechange, near line 120, where var update = JSON.parse... can be read, add a reject('foo'); call
  8. Reveal file in sidebar, right click it and choose 'save for overrides'
@Siegrift
Copy link

Siegrift commented Jul 5, 2021

I am experiencing the same issues in my project: https://github.com/api3dao/api3-dao-dashboard

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 9, 2022
@Sidnioulz
Copy link
Author

Sidnioulz commented Jan 10, 2022

Too bad the issue was not triaged in time. As it turns out, someone else has rewritten the handleApplyUpdates function on main and it's likely the bug is fixed, based on a cursory read of the new logic.

And so: Fixed by #11105.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants