From e976b065628ca451cd07bf5472427462d64bc56c Mon Sep 17 00:00:00 2001 From: Ian VanSchooten Date: Wed, 5 Oct 2022 05:24:32 -0400 Subject: [PATCH] fix(react): conditionally self-accept fast-refresh HMR (#10239) Co-authored-by: Alec Larson <1925840+aleclarson@users.noreply.github.com> --- packages/plugin-react/src/fast-refresh.ts | 54 ++++++++++++++++--- playground/react/App.jsx | 24 +++++++-- playground/react/__tests__/react.spec.ts | 62 +++++++++++++++++++--- playground/react/context/ContextButton.jsx | 11 ++++ playground/react/context/CountProvider.jsx | 12 +++++ playground/react/hmr/no-exported-comp.jsx | 7 +++ playground/react/hmr/parent.jsx | 7 +++ 7 files changed, 160 insertions(+), 17 deletions(-) create mode 100644 playground/react/context/ContextButton.jsx create mode 100644 playground/react/context/CountProvider.jsx create mode 100644 playground/react/hmr/no-exported-comp.jsx create mode 100644 playground/react/hmr/parent.jsx diff --git a/packages/plugin-react/src/fast-refresh.ts b/packages/plugin-react/src/fast-refresh.ts index b3b095a65cf2ae..b0b38a8cafb94e 100644 --- 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 index 3ec29ba38d893b..83f4cc07ea4a07 100644 --- 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() {

Hello Vite + React

-

+

+ +

Edit App.jsx and save to test HMR updates.

@@ -27,9 +36,18 @@ function App() {
+ ) } -export default App +function AppWithProviders() { + return ( + + + + ) +} + +export default AppWithProviders diff --git a/playground/react/__tests__/react.spec.ts b/playground/react/__tests__/react.spec.ts index 15f6319220d7f2..654a45a668e894 100644 --- 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 new file mode 100644 index 00000000000000..92c6d0bd26f968 --- /dev/null +++ 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 ( + + ) +} diff --git a/playground/react/context/CountProvider.jsx b/playground/react/context/CountProvider.jsx new file mode 100644 index 00000000000000..223ad25f04f056 --- /dev/null +++ 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 ( + + {children} +
context provider
+ + ) +} diff --git a/playground/react/hmr/no-exported-comp.jsx b/playground/react/hmr/no-exported-comp.jsx new file mode 100644 index 00000000000000..7784bcb50603a9 --- /dev/null +++ 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 = () => An unused react component + +export const Foo = { + is: 'An Object' +} diff --git a/playground/react/hmr/parent.jsx b/playground/react/hmr/parent.jsx new file mode 100644 index 00000000000000..ff8698281c83c7 --- /dev/null +++ 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
{Foo.is}
+}