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

fix: fast refresh stops on needed bail outs #11105

Merged
merged 2 commits into from
Jul 14, 2021
Merged

fix: fast refresh stops on needed bail outs #11105

merged 2 commits into from
Jul 14, 2021

Conversation

pmmmwh
Copy link
Contributor

@pmmmwh pmmmwh commented Jun 15, 2021

Fast Refresh requires the HMR runtime to support bail out behaviour (we do not do so within the core runtime as it has to be platform agnostic) - and currently webpackHotDevClient does not do so properly as it circumvents the logic for forced reloads completely when using Fast Refresh.

The changes done here ensures that:

  • If Fast Refresh is not enabled, we would always bail out to a forced reload;
  • If Fast Refresh is enabled and there are updated modules, it indicates the update has at least partially executed, and we can rely on Fast Refresh being resilient to errors and skip the forced reload;
  • If Fast Refresh is enabled and there are none updated modules, check for the status of the hot update. If it is abort or failed, it indicates the update cannot be executed without being inconsistent (i.e. Fast Refresh bailed out), we would bail out to a forced reload.

To verify the impact of this change:

To verify addendum to this change (error recovery):

  • Using the same project, remove extra exports in App.js
  • Run the project, wait for the first compilation to complete
  • Try adding a throw new Error('no') within the App component, compilation should show the error overlay
  • Try removing the error within the component, App should HMR properly without forced refresh
  • Try creating syntax errors within the module (e.g. remove quotation marks in imports), compilation should show the error overlay
  • Try fixing the error, App should HMR properly without forced refresh

This should:
Fixes #9904
Fixes #9913
Fixes #9984
Fixes Partially #10078
Fixes #10287
Fixes #10539
Fixes #10606
Fixes #11087

Fast Refresh requires the HMR runtime to support bail out behaviour (we do not do so within the core runtime as it has to be platform agnostic) - and currently webpackHotDevClient does not do so properly as it circumvents the logic for forced reloads completely when using Fast Refresh.

The changes done here ensures that:
- If Fast Refresh is not enabled, we would always bail out to a forced reload;
- If Fast Refresh is enabled and there are updated modules, it indicates the update has at least partially executed, and we can rely on Fast Refresh being resilient to errors and skip the forced reload;
- If Fast Refresh is enabled and there are none updated modules, it indicates the update cannot be executed without being inconsistent (i.e. Fast Refresh bailed out), we would bail out to a forced reload.
@pmmmwh
Copy link
Contributor Author

pmmmwh commented Jun 15, 2021

Updated heuristic to cover more cases (to be on the safe side) where we would force a reload, especially when Webpack signifies that the HMR update is aborted or failed completely (indicating applying the update would yield inconsistency).

Copy link
Contributor

@mrmckeb mrmckeb left a comment

Choose a reason for hiding this comment

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

I haven't tested yet, but this sounds great. We'll try to get this into the next release - thanks @pmmmwh!

@iansu
Copy link
Contributor

iansu commented Jul 14, 2021

Thanks! Looking forward to getting these enhancements out in v5!

@mrmckeb mrmckeb mentioned this pull request Jul 14, 2021
@pmmmwh pmmmwh deleted the patch-1 branch July 14, 2021 17:20
sumanthratna pushed a commit to sumanthratna/create-react-app that referenced this pull request Aug 4, 2021
abhiisheek pushed a commit to abhiisheek/create-react-app that referenced this pull request May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants