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

Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com>
vitejs · Oct 5, 2022 · e976b06 · e976b06
1 parent cba13e8
commit e976b06
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 17 deletions.
diff --git a/packages/plugin-react/src/fast-refresh.ts b/packages/plugin-react/src/fast-refresh.ts
@@ -58,20 +58,57 @@ if (import.meta.hot) {
   window.$RefreshSig$ = RefreshRuntime.createSignatureFunctionForTransform;
 }`.replace(/[\n]+/gm, '')
 
-const footer = `
-if (import.meta.hot) {
-  window.$RefreshReg$ = prevRefreshReg;
-  window.$RefreshSig$ = prevRefreshSig;
-
-  __ACCEPT__
+const timeout = `
   if (!window.__vite_plugin_react_timeout) {
     window.__vite_plugin_react_timeout = setTimeout(() => {
       window.__vite_plugin_react_timeout = 0;
       RefreshRuntime.performReactRefresh();
     }, 30);
   }
+`
+
+const footer = `
+if (import.meta.hot) {
+  window.$RefreshReg$ = prevRefreshReg;
+  window.$RefreshSig$ = prevRefreshSig;
+
+  __ACCEPT__
 }`
 
+const checkAndAccept = `
+function isReactRefreshBoundary(mod) {
+  if (mod == null || typeof mod !== 'object') {
+    return false;
+  }
+  let hasExports = false;
+  let areAllExportsComponents = true;
+  for (const exportName in mod) {
+    hasExports = true;
+    if (exportName === '__esModule') {
+      continue;
+    }
+    const desc = Object.getOwnPropertyDescriptor(mod, exportName);
+    if (desc && desc.get) {
+      // Don't invoke getters as they may have side effects.
+      return false;
+    }
+    const exportValue = mod[exportName];
+    if (!RefreshRuntime.isLikelyComponentType(exportValue)) {
+      areAllExportsComponents = false;
+    }
+  }
+  return hasExports && areAllExportsComponents;
+}
+
+import.meta.hot.accept(mod => {
+  if (isReactRefreshBoundary(mod)) {
+    ${timeout}
+  } else {
+    import.meta.hot.invalidate();
+  }
+});
+`
+
 export function addRefreshWrapper(
   code: string,
   id: string,
@@ -80,12 +117,13 @@ export function addRefreshWrapper(
   return (
     header.replace('__SOURCE__', JSON.stringify(id)) +
     code +
-    footer.replace('__ACCEPT__', accept ? 'import.meta.hot.accept();' : '')
+    footer.replace('__ACCEPT__', accept ? checkAndAccept : timeout)
   )
 }
 
 export function isRefreshBoundary(ast: t.File): boolean {
-  // Every export must be a React component.
+  // Every export must be a potential React component.
+  // We'll also perform a runtime check that's more robust as well (isLikelyComponentType).
   return ast.program.body.every((node) => {
     if (node.type !== 'ExportNamedDeclaration') {
       return true

diff --git a/playground/react/App.jsx b/playground/react/App.jsx
@@ -1,6 +1,9 @@
 import { useState } from 'react'
-import Dummy from './components/Dummy?qs-should-not-break-plugin-react'
 import Button from 'jsx-entry'
+import Dummy from './components/Dummy?qs-should-not-break-plugin-react'
+import Parent from './hmr/parent'
+import { CountProvider } from './context/CountProvider'
+import { ContextButton } from './context/ContextButton'
 
 function App() {
   const [count, setCount] = useState(0)
@@ -9,10 +12,16 @@ function App() {
       <header className="App-header">
         <h1>Hello Vite + React</h1>
         <p>
-          <button onClick={() => setCount((count) => count + 1)}>
+          <button
+            id="state-button"
+            onClick={() => setCount((count) => count + 1)}
+          >
             count is: {count}
           </button>
         </p>
+        <p>
+          <ContextButton />
+        </p>
         <p>
           Edit <code>App.jsx</code> and save to test HMR updates.
         </p>
@@ -27,9 +36,18 @@ function App() {
       </header>
 
       <Dummy />
+      <Parent />
       <Button>button</Button>
     </div>
   )
 }
 
-export default App
+function AppWithProviders() {
+  return (
+    <CountProvider>
+      <App />
+    </CountProvider>
+  )
+}
+
+export default AppWithProviders
diff --git a/playground/react/__tests__/react.spec.ts b/playground/react/__tests__/react.spec.ts
@@ -1,28 +1,35 @@
 import { expect, test } from 'vitest'
-import { editFile, isServe, page, untilUpdated } from '~utils'
+import {
+  browserLogs,
+  editFile,
+  isBuild,
+  isServe,
+  page,
+  untilUpdated
+} from '~utils'
 
 test('should render', async () => {
   expect(await page.textContent('h1')).toMatch('Hello Vite + React')
 })
 
 test('should update', async () => {
-  expect(await page.textContent('button')).toMatch('count is: 0')
-  await page.click('button')
-  expect(await page.textContent('button')).toMatch('count is: 1')
+  expect(await page.textContent('#state-button')).toMatch('count is: 0')
+  await page.click('#state-button')
+  expect(await page.textContent('#state-button')).toMatch('count is: 1')
 })
 
 test('should hmr', async () => {
   editFile('App.jsx', (code) => code.replace('Vite + React', 'Updated'))
   await untilUpdated(() => page.textContent('h1'), 'Hello Updated')
   // preserve state
-  expect(await page.textContent('button')).toMatch('count is: 1')
+  expect(await page.textContent('#state-button')).toMatch('count is: 1')
 })
 
 test.runIf(isServe)(
   'should have annotated jsx with file location metadata',
   async () => {
     const meta = await page.evaluate(() => {
-      const button = document.querySelector('button')
+      const button = document.querySelector('#state-button')
       const key = Object.keys(button).find(
         (key) => key.indexOf('__reactFiber') === 0
       )
@@ -37,3 +44,46 @@ test.runIf(isServe)(
     ])
   }
 )
+
+if (!isBuild) {
+  // #9869
+  test('should only hmr files with exported react components', async () => {
+    browserLogs.length = 0
+    editFile('hmr/no-exported-comp.jsx', (code) =>
+      code.replace('An Object', 'Updated')
+    )
+    await untilUpdated(() => page.textContent('#parent'), 'Updated')
+    expect(browserLogs).toMatchObject([
+      '[vite] hot updated: /hmr/no-exported-comp.jsx',
+      '[vite] hot updated: /hmr/parent.jsx',
+      'Parent rendered'
+    ])
+    browserLogs.length = 0
+  })
+
+  // #3301
+  test('should hmr react context', async () => {
+    browserLogs.length = 0
+    expect(await page.textContent('#context-button')).toMatch(
+      'context-based count is: 0'
+    )
+    await page.click('#context-button')
+    expect(await page.textContent('#context-button')).toMatch(
+      'context-based count is: 1'
+    )
+    editFile('context/CountProvider.jsx', (code) =>
+      code.replace('context provider', 'context provider updated')
+    )
+    await untilUpdated(
+      () => page.textContent('#context-provider'),
+      'context provider updated'
+    )
+    expect(browserLogs).toMatchObject([
+      '[vite] hot updated: /context/CountProvider.jsx',
+      '[vite] hot updated: /App.jsx',
+      '[vite] hot updated: /context/ContextButton.jsx',
+      'Parent rendered'
+    ])
+    browserLogs.length = 0
+  })
+}
diff --git a/playground/react/context/ContextButton.jsx b/playground/react/context/ContextButton.jsx
@@ -0,0 +1,11 @@
+import { useContext } from 'react'
+import { CountContext } from './CountProvider'
+
+export function ContextButton() {
+  const { count, setCount } = useContext(CountContext)
+  return (
+    <button id="context-button" onClick={() => setCount((count) => count + 1)}>
+      context-based count is: {count}
+    </button>
+  )
+}
diff --git a/playground/react/context/CountProvider.jsx b/playground/react/context/CountProvider.jsx
@@ -0,0 +1,12 @@
+import { createContext, useState } from 'react'
+export const CountContext = createContext()
+
+export const CountProvider = ({ children }) => {
+  const [count, setCount] = useState(0)
+  return (
+    <CountContext.Provider value={{ count, setCount }}>
+      {children}
+      <div id="context-provider">context provider</div>
+    </CountContext.Provider>
+  )
+}
diff --git a/playground/react/hmr/no-exported-comp.jsx b/playground/react/hmr/no-exported-comp.jsx
@@ -0,0 +1,7 @@
+// This un-exported react component should not cause this file to be treated
+// as an HMR boundary
+const Unused = () => <span>An unused react component</span>
+
+export const Foo = {
+  is: 'An Object'
+}
diff --git a/playground/react/hmr/parent.jsx b/playground/react/hmr/parent.jsx
@@ -0,0 +1,7 @@
+import { Foo } from './no-exported-comp'
+
+export default function Parent() {
+  console.log('Parent rendered')
+
+  return <div id="parent">{Foo.is}</div>
+}