Skip to content

fix(react): conditionally self-accept fast-refresh HMR#10239

Merged
patak-cat merged 8 commits intovitejs:mainfrom
IanVS:issue/9869-fast-refresh-boundaries
Oct 5, 2022
Merged

fix(react): conditionally self-accept fast-refresh HMR#10239
patak-cat merged 8 commits intovitejs:mainfrom
IanVS:issue/9869-fast-refresh-boundaries

Conversation

@IanVS
Copy link
Copy Markdown
Contributor

@IanVS IanVS commented Sep 25, 2022

Description

Fixes #9869
Fixes #3301
Fixes #4298
Fixes #6885
Fixes #7396

Based on top of #10244

This makes a change to the HMR code that's injected by plugin-react to check the exports of a module, and determine whether it is safe to treat as a fast-refresh boundary. If it is a safe fast-refresh boundary, we will perform fast-refresh. If not, we now call hot.invalidate() (relying on the changes in #10244) to propagate HMR up to parents. This is necessary because vite sees import.meta.hot.accept() even if it's inside a conditional, and treats the module as self-accepting. So without calling hot.invalidate(), the parent will never know to fast-refresh.

I've also added a test here for changing a react context file, and verified that it fails without the changes here, but passes with them.

Additional context

The other PR, #10244 should be reviewed first, since this one depends on it.

Thanks for help on this, @aleclarson!

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes vitejs/vite#123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@IanVS IanVS changed the title [Plugin-React] WIP: Conditionally self-accept HMR fix: Conditionally self-accept fast-refresh HMR [WIP] Sep 25, 2022
@aleclarson
Copy link
Copy Markdown
Contributor

aleclarson commented Sep 25, 2022

I pushed a commit here that implements the hot.invalidate API (a45385f), but it probably deserves its own PR. If anyone wants to push that forward, please do. I've no time currently.

Todo

  • Figure out how to avoid calling hot.invalidate on first page load, or have the dev server detect it as such and do nothing in response.

@IanVS IanVS force-pushed the issue/9869-fast-refresh-boundaries branch from a94c657 to 37dbbd0 Compare September 26, 2022 01:49
@IanVS IanVS changed the title fix: Conditionally self-accept fast-refresh HMR [WIP] fix: Conditionally self-accept fast-refresh HMR Sep 26, 2022
@IanVS IanVS force-pushed the issue/9869-fast-refresh-boundaries branch from 56f52e1 to 5d6b80e Compare September 26, 2022 13:51
@IanVS IanVS force-pushed the issue/9869-fast-refresh-boundaries branch from 5d6b80e to 535aa4f Compare September 28, 2022 21:33
@IanVS
Copy link
Copy Markdown
Contributor Author

IanVS commented Sep 28, 2022

@aleclarson I've rebased this onto main now that the hmr.invalidate() PR is in. Thanks again for all your help on this.

@sapphi-red sapphi-red changed the title fix: Conditionally self-accept fast-refresh HMR fix: conditionally self-accept fast-refresh HMR Oct 5, 2022
@sapphi-red sapphi-red changed the title fix: conditionally self-accept fast-refresh HMR fix(react): conditionally self-accept fast-refresh HMR Oct 5, 2022
@sapphi-red
Copy link
Copy Markdown
Member

@sapphi-red sapphi-red added the p4-important Violate documented behavior or significantly improves performance (priority) label Oct 5, 2022
@patak-cat patak-cat merged commit e976b06 into vitejs:main Oct 5, 2022
@IanVS IanVS deleted the issue/9869-fast-refresh-boundaries branch October 5, 2022 12:04
@abouroubi
Copy link
Copy Markdown

Thanks for the great job ! I think this was a blocking issue for a lot of us React users.

There is a small issue with the CHANGELOG as this PR is not referenced there (even if it's int he commit history of release: v3.2.0-beta.1).
Regarding the number of people waiting for this fix, I think it would be a good idea to mention it.

@IanVS
Copy link
Copy Markdown
Contributor Author

IanVS commented Oct 10, 2022

Hi @abouroubi, this change was made to @vitejs/plugin-react, so the changelog is in that project: https://github.com/vitejs/vite/blob/plugin-react@2.2.0-beta.0/packages/plugin-react/CHANGELOG.md.

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

Labels

feat: hmr p4-important Violate documented behavior or significantly improves performance (priority)

Projects

None yet

5 participants