diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 83b12a04a799..b03e806fc2d2 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -79,11 +79,7 @@ import { } from './ReactWorkTags'; import {getComponentNameFromOwner} from 'react-reconciler/src/getComponentNameFromFiber'; import {isDevToolsPresent} from './ReactFiberDevToolsHook'; -import { - resolveClassForHotReloading, - resolveFunctionForHotReloading, - resolveForwardRefForHotReloading, -} from './ReactFiberHotReloading'; +import {resolveTypeForHotReloading} from './ReactFiberHotReloading'; import {NoLanes} from './ReactFiberLane'; import { NoMode, @@ -427,13 +423,10 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { switch (workInProgress.tag) { case FunctionComponent: case SimpleMemoComponent: - workInProgress.type = resolveFunctionForHotReloading(current.type); - break; + case MemoComponent: case ClassComponent: - workInProgress.type = resolveClassForHotReloading(current.type); - break; case ForwardRef: - workInProgress.type = resolveForwardRefForHotReloading(current.type); + workInProgress.type = resolveTypeForHotReloading(current.type); break; default: break; @@ -569,18 +562,14 @@ export function createFiberFromTypeAndProps( let fiberTag: WorkTag = FunctionComponent; // The resolved type is set if we know what the final type will be. I.e. it's not lazy. let resolvedType = type; - if (typeof type === 'function') { - if (shouldConstruct(type)) { + if (__DEV__) { + resolvedType = resolveTypeForHotReloading(type); + } + if (typeof resolvedType === 'function') { + if (shouldConstruct(resolvedType)) { fiberTag = ClassComponent; - if (__DEV__) { - resolvedType = resolveClassForHotReloading(resolvedType); - } - } else { - if (__DEV__) { - resolvedType = resolveFunctionForHotReloading(resolvedType); - } } - } else if (typeof type === 'string') { + } else if (typeof resolvedType === 'string') { // $FlowFixMe[constant-condition] if (supportsResources && supportsSingletons) { const hostContext = getHostContext(); @@ -602,7 +591,7 @@ export function createFiberFromTypeAndProps( fiberTag = HostComponent; } } else { - getTag: switch (type) { + getTag: switch (resolvedType) { // $FlowFixMe[invalid-compare] case REACT_ACTIVITY_TYPE: return createFiberFromActivity(pendingProps, mode, lanes, key); @@ -650,8 +639,8 @@ export function createFiberFromTypeAndProps( // Fall through default: { // $FlowFixMe[invalid-compare] - if (typeof type === 'object' && type !== null) { - switch (type.$$typeof) { + if (typeof resolvedType === 'object' && resolvedType !== null) { + switch (resolvedType.$$typeof) { // $FlowFixMe[invalid-compare] case REACT_CONTEXT_TYPE: fiberTag = ContextProvider; @@ -664,9 +653,6 @@ export function createFiberFromTypeAndProps( // $FlowFixMe[invalid-compare] case REACT_FORWARD_REF_TYPE: fiberTag = ForwardRef; - if (__DEV__) { - resolvedType = resolveForwardRefForHotReloading(resolvedType); - } break getTag; // $FlowFixMe[invalid-compare] case REACT_MEMO_TYPE: diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index ed040e6d35a3..815dc8b99612 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -132,12 +132,7 @@ import { REACT_CONTEXT_TYPE, } from 'shared/ReactSymbols'; import {setCurrentFiber} from './ReactCurrentFiber'; -import { - resolveFunctionForHotReloading, - resolveForwardRefForHotReloading, - resolveClassForHotReloading, - resolveRemountTypeForHotReloading, -} from './ReactFiberHotReloading'; +import {resolveTypeForHotReloading} from './ReactFiberHotReloading'; import { mountChildFibers, @@ -414,7 +409,16 @@ function updateForwardRef( // TODO: current can be non-null here even if the component // hasn't yet mounted. This happens after the first render suspends. // We'll need to figure out if this is fine or can cause issues. - const render = Component.render; + let render = Component.render; + if (__DEV__) { + const resolvedRender = resolveTypeForHotReloading(render); + if (resolvedRender !== render) { + render = resolvedRender; + if (current !== null) { + didReceiveUpdate = true; + } + } + } const ref = workInProgress.ref; let propsWithoutRef; @@ -482,7 +486,7 @@ function updateMemoComponent( if (isSimpleFunctionComponent(type) && Component.compare === null) { let resolvedType = type; if (__DEV__) { - resolvedType = resolveFunctionForHotReloading(type); + resolvedType = resolveTypeForHotReloading(type); } // If this is a plain function component without default props, // and with only the default shallow comparison, we upgrade it @@ -2095,6 +2099,9 @@ function mountLazyComponent( const props = workInProgress.pendingProps; const lazyComponent: LazyComponentType = elementType; let Component = resolveLazy(lazyComponent); + if (__DEV__) { + Component = resolveTypeForHotReloading(Component); + } // Store the unwrapped component in the type. workInProgress.type = Component; @@ -2102,10 +2109,6 @@ function mountLazyComponent( if (isFunctionClassComponent(Component)) { const resolvedProps = resolveClassComponentProps(Component, props); workInProgress.tag = ClassComponent; - if (__DEV__) { - workInProgress.type = Component = - resolveClassForHotReloading(Component); - } return updateClassComponent( null, workInProgress, @@ -2117,8 +2120,6 @@ function mountLazyComponent( workInProgress.tag = FunctionComponent; if (__DEV__) { validateFunctionComponentInDev(workInProgress, Component); - workInProgress.type = Component = - resolveFunctionForHotReloading(Component); } return updateFunctionComponent( null, @@ -2134,10 +2135,6 @@ function mountLazyComponent( // $FlowFixMe[invalid-compare] if ($$typeof === REACT_FORWARD_REF_TYPE) { workInProgress.tag = ForwardRef; - if (__DEV__) { - workInProgress.type = Component = - resolveForwardRefForHotReloading(Component); - } return updateForwardRef( null, workInProgress, @@ -4196,10 +4193,9 @@ function beginWork( if (workInProgress._debugNeedsRemount && current !== null) { // This will restart the begin phase with a new fiber. const copiedFiber = createFiberFromTypeAndProps( - resolveRemountTypeForHotReloading( - workInProgress.elementType, - workInProgress.type, - ), + // Remount from the fiber's outermost identity; mounting resolves + // any inner types to their latest implementations. + resolveTypeForHotReloading(workInProgress.elementType), workInProgress.key, workInProgress.pendingProps, workInProgress._debugOwner || null, diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.js b/packages/react-reconciler/src/ReactFiberHotReloading.js index 199e7486f1c6..16c30f3724d5 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.js @@ -61,7 +61,7 @@ export const setRefreshHandler = (handler: RefreshHandler | null): void => { } }; -export function resolveFunctionForHotReloading(type: any): any { +export function resolveTypeForHotReloading(type: any): any { if (__DEV__) { if (resolveFamily === null) { // Hot reloading is disabled. @@ -78,72 +78,6 @@ export function resolveFunctionForHotReloading(type: any): any { } } -export function resolveClassForHotReloading(type: any): any { - // No implementation differences. - return resolveFunctionForHotReloading(type); -} - -export function resolveForwardRefForHotReloading(type: any): any { - if (__DEV__) { - if (resolveFamily === null) { - // Hot reloading is disabled. - return type; - } - const family = resolveFamily(type); - if (family === undefined) { - // Check if we're dealing with a real forwardRef. Don't want to crash early. - if ( - type !== null && - type !== undefined && - typeof type.render === 'function' - ) { - // ForwardRef is special because its resolved .type is an object, - // but it's possible that we only have its inner render function in the map. - // If that inner render function is different, we'll build a new forwardRef type. - const currentRender = resolveFunctionForHotReloading(type.render); - if (type.render !== currentRender) { - const syntheticType = { - $$typeof: REACT_FORWARD_REF_TYPE, - render: currentRender, - }; - if (type.displayName !== undefined) { - (syntheticType as any).displayName = type.displayName; - } - return syntheticType; - } - } - return type; - } - // Use the latest known implementation. - return family.current; - } else { - return type; - } -} - -export function resolveRemountTypeForHotReloading( - elementType: any, - type: any, -): any { - if (__DEV__) { - if (resolveFamily === null) { - // Hot reloading is disabled. - return type; - } - // The elementType is the fiber's public identity, so its family tracks - // the latest implementation even when an edit changed the kind of the - // type (e.g. memo to a plain function) and `type` still points at the - // old inner implementation. - const family = resolveFamily(elementType); - if (family === undefined) { - return type; - } - return family.current; - } else { - return type; - } -} - export function isCompatibleFamilyForHotReloading( fiber: Fiber, element: ReactElement, @@ -196,8 +130,6 @@ export function isCompatibleFamilyForHotReloading( case MemoComponent: case SimpleMemoComponent: { if ($$typeofNextType === REACT_MEMO_TYPE) { - // TODO: if it was but can no longer be simple, - // we shouldn't set this. needsCompareFamilies = true; } else if ($$typeofNextType === REACT_LAZY_TYPE) { needsCompareFamilies = true; @@ -339,6 +271,17 @@ function scheduleFibersWithFamiliesRecursively( const outerFamily = resolve(outerCandidateType); if (outerFamily !== undefined && staleFamilies.has(outerFamily)) { needsRemount = true; + } else if ( + typeof outerCandidateType === 'object' && + outerCandidateType.$$typeof === REACT_LAZY_TYPE + ) { + const payload = outerCandidateType._payload; + if (payload._status === 1 /* Resolved; see ReactLazy */) { + const middleFamily = resolve(payload._result.default); + if (middleFamily !== undefined && staleFamilies.has(middleFamily)) { + needsRemount = true; + } + } } } if (failedBoundaries !== null) { diff --git a/packages/react-refresh/src/ReactFreshRuntime.js b/packages/react-refresh/src/ReactFreshRuntime.js index 1f4944d6992d..f02215b0c858 100644 --- a/packages/react-refresh/src/ReactFreshRuntime.js +++ b/packages/react-refresh/src/ReactFreshRuntime.js @@ -158,6 +158,16 @@ function canPreserveStateBetween(prevType: any, nextType: any) { ) { return false; } + // Switching from SimpleMemoComponent to MemoComponent requires a remount; + // for symmetry, remount for the reverse too. + if (getProperty(prevType, '$$typeof') === REACT_MEMO_TYPE) { + if ( + (getProperty(prevType, 'compare') === null) !== + (getProperty(nextType, 'compare') === null) + ) { + return false; + } + } } if (haveEqualSignatures(prevType, nextType)) { return true; @@ -187,7 +197,7 @@ function cloneSet(set: Set): Set { } // This is a safety mechanism to protect against rogue getters and Proxies. -function getProperty(object: any, property: string) { +function getProperty(object: any, property: string): any { try { return object[property]; } catch (err) { diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js index 84f5e9a944c3..53a71538620d 100644 --- a/packages/react-refresh/src/__tests__/ReactFresh-test.js +++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js @@ -915,6 +915,460 @@ describe('ReactFresh', () => { } }); + it('can mount an element created before its type changed kinds', async () => { + if (__DEV__) { + let oldElement; + let currentChild = null; + + await act(async () => { + await render(() => { + function Test() { + return

hi test

; + } + $RefreshReg$(Test, 'Test'); + oldElement = ; + + function App() { + const [, forceUpdate] = React.useState(0); + return ( +
forceUpdate(n => n + 1)}>{currentChild}
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // Change the component kind before it has ever mounted. + await act(async () => { + await patch(() => { + function Test2() { + return

hi memo

; + } + const Test = React.memo(Test2); + $RefreshReg$(Test2, 'Test$React.memo'); + $RefreshReg$(Test, 'Test'); + + function App() { + const [, forceUpdate] = React.useState(0); + return ( +
forceUpdate(n => n + 1)}>{currentChild}
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // Mount the element created before the edit. The fiber must be + // created from the latest type, with the tag matching its kind. + currentChild = oldElement; + await act(async () => { + container.firstChild.click(); + }); + expect(container.firstChild.textContent).toBe('hi memo'); + } + }); + + it('can remount when adding or removing a memo comparison function', async () => { + if (__DEV__) { + await act(async () => { + await render(() => { + function Test2() { + return

hi memo

; + } + const Test = React.memo(Test2); + $RefreshReg$(Test2, 'Test$React.memo'); + $RefreshReg$(Test, 'Test'); + return Test; + }); + }); + + // Check the initial render + const el = container.firstChild; + expect(el.textContent).toBe('hi memo'); + + // Patch to add a custom comparison function. + // The fiber can no longer be a SimpleMemoComponent. + await act(async () => { + await patch(() => { + function Test2() { + return

hi memo with compare

; + } + const Test = React.memo(Test2, (prevProps, nextProps) => false); + $RefreshReg$(Test2, 'Test$React.memo'); + $RefreshReg$(Test, 'Test'); + return Test; + }); + }); + + // Check remount + expect(container.firstChild).not.toBe(el); + const nextEl = container.firstChild; + expect(nextEl.textContent).toBe('hi memo with compare'); + + // Patch to remove the comparison function again + await act(async () => { + await patch(() => { + function Test2() { + return

hi memo

; + } + const Test = React.memo(Test2); + $RefreshReg$(Test2, 'Test$React.memo'); + $RefreshReg$(Test, 'Test'); + return Test; + }); + }); + + // Check final remount + expect(container.firstChild).not.toBe(nextEl); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('hi memo'); + } + }); + + it('can update a memo comparison function in place', async () => { + if (__DEV__) { + await act(async () => { + await render(() => { + function Inner({label}) { + return

{label}

; + } + const InnerMemo = React.memo(Inner, (prevProps, nextProps) => true); + $RefreshReg$(Inner, 'Inner$React.memo'); + $RefreshReg$(InnerMemo, 'Inner'); + + function App() { + const [n, setN] = React.useState(1); + return ( +
setN(c => c + 1)}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // Check the initial render + const el = container.firstChild; + expect(el.textContent).toBe('n:1'); + + // The comparison function blocks the update. + await act(async () => { + el.click(); + }); + expect(el.textContent).toBe('n:1'); + + // Patch to change only the comparison function implementation. + await act(async () => { + await patch(() => { + function Inner({label}) { + return

{label}

; + } + const InnerMemo = React.memo(Inner, (prevProps, nextProps) => false); + $RefreshReg$(Inner, 'Inner$React.memo'); + $RefreshReg$(InnerMemo, 'Inner'); + + function App() { + const [n, setN] = React.useState(1); + return ( +
setN(c => c + 1)}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // No remount, and the previously blocked update shows through + // because the new comparison function is used. + expect(container.firstChild).toBe(el); + expect(el.textContent).toBe('n:2'); + + // The new comparison function applies to future updates too. + await act(async () => { + el.click(); + }); + expect(el.textContent).toBe('n:3'); + } + }); + + it('mounts a pre-edit memo element with the latest comparison function', async () => { + if (__DEV__) { + let oldElement; + let newElement; + let currentChild = null; + + await act(async () => { + await render(() => { + function Inner({label}) { + return

{label}

; + } + const InnerMemo = React.memo(Inner); + $RefreshReg$(Inner, 'Inner$React.memo'); + $RefreshReg$(InnerMemo, 'Inner'); + oldElement = ; + + function App() { + const [, forceUpdate] = React.useState(0); + return ( +
forceUpdate(n => n + 1)}>{currentChild}
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // Patch to add a comparison function that blocks all updates, + // before the memo has ever mounted. + await act(async () => { + await patch(() => { + function Inner({label}) { + return

{label}

; + } + const InnerMemo = React.memo(Inner, (prevProps, nextProps) => true); + $RefreshReg$(Inner, 'Inner$React.memo'); + $RefreshReg$(InnerMemo, 'Inner'); + newElement = ; + + function App() { + const [, forceUpdate] = React.useState(0); + return ( +
forceUpdate(n => n + 1)}>{currentChild}
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + // Mount the element created before the edit. It must resolve to the + // latest type rather than mounting in the pre-edit shape. + currentChild = oldElement; + await act(async () => { + container.firstChild.click(); + }); + const innerEl = container.firstChild.firstChild; + expect(innerEl.textContent).toBe('v1'); + + // Switch to the element created after the edit. It belongs to the + // same family, so the fiber is reused (no remount)... + currentChild = newElement; + await act(async () => { + container.firstChild.click(); + }); + expect(container.firstChild.firstChild).toBe(innerEl); + // ...and the comparison function blocks the props update, proving + // the fiber mounted with the comparison function in effect. + expect(innerEl.textContent).toBe('v1'); + } + }); + + it('can remount lazy(memo()) when adding a comparison function', async () => { + if (__DEV__) { + let resolve; + await render(() => { + function Hello() { + return

hi memo

; + } + const Inner = React.memo(Hello); + $RefreshReg$(Hello, 'Hello'); + $RefreshReg$(Inner, 'Inner'); + + const Outer = React.lazy( + () => + new Promise(_resolve => { + resolve = () => _resolve({default: Inner}); + }), + ); + $RefreshReg$(Outer, 'Outer'); + + function App() { + return ( + Loading

}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + + expect(container.textContent).toBe('Loading'); + await act(() => { + resolve(); + }); + expect(container.textContent).toBe('hi memo'); + const el = container.firstChild; + + // Perform a hot update that adds a comparison function. The module + // creating the lazy also re-runs, like when an edit propagates. + await patch(() => { + function Hello() { + return

hi memo with compare

; + } + const Inner = React.memo(Hello, (prevProps, nextProps) => false); + $RefreshReg$(Hello, 'Hello'); + $RefreshReg$(Inner, 'Inner'); + + const Outer = React.lazy( + () => + new Promise(_resolve => { + resolve = () => _resolve({default: Inner}); + }), + ); + $RefreshReg$(Outer, 'Outer'); + + function App() { + return ( + Loading

}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + + // The shape change requires a remount. It goes through the latest + // lazy type, which suspends until it resolves. The boundary shows + // the fallback while the previous content stays hidden in the DOM. + expect(container.textContent).toBe('hi memoLoading'); + await act(() => { + resolve(); + }); + expect(container.textContent).toBe('hi memo with compare'); + expect(container.firstChild).not.toBe(el); + } + }); + + it('can remount lazy(memo()) when adding a comparison function without re-creating the lazy', async () => { + if (__DEV__) { + let resolve; + await render(() => { + function Hello() { + return

hi memo

; + } + const Inner = React.memo(Hello); + $RefreshReg$(Hello, 'Hello'); + $RefreshReg$(Inner, 'Inner'); + + const Outer = React.lazy( + () => + new Promise(_resolve => { + resolve = () => _resolve({default: Inner}); + }), + ); + $RefreshReg$(Outer, 'Outer'); + + function App() { + return ( + Loading

}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + + expect(container.textContent).toBe('Loading'); + await act(() => { + resolve(); + }); + expect(container.textContent).toBe('hi memo'); + const el = container.firstChild; + + // Only the lazily loaded module re-runs this time, like when an + // edit is contained to it (the lazy type is not re-created, so the + // remount cannot go through it). + await patch(() => { + function Hello() { + return

hi memo with compare

; + } + const Inner = React.memo(Hello, (prevProps, nextProps) => false); + $RefreshReg$(Hello, 'Hello'); + $RefreshReg$(Inner, 'Inner'); + return Inner; + }); + + // The remount goes through the old lazy, whose payload is already + // resolved, so it doesn't suspend. + expect(container.textContent).toBe('hi memo with compare'); + expect(container.firstChild).not.toBe(el); + } + }); + + it('can remount an unregistered memo wrapper without losing the wrapper', async () => { + if (__DEV__) { + let innerRenders = 0; + await act(async () => { + await render(() => { + function Inner({label}) { + innerRenders++; + return

{label}

; + } + $RefreshReg$(Inner, 'Inner'); + $RefreshSig$(Inner, 'sig1'); + // The wrapper is deliberately not registered, like a wrapper + // created inside a third-party HOC. + const InnerMemo = React.memo(Inner); + + function App() { + const [, forceUpdate] = React.useState(0); + return ( +
forceUpdate(n => n + 1)}> + +
+ ); + } + $RefreshReg$(App, 'App'); + return App; + }); + }); + + expect(container.textContent).toBe('hi'); + expect(innerRenders).toBe(1); + + // The memo blocks re-renders with equal props. + await act(async () => { + container.firstChild.click(); + }); + expect(innerRenders).toBe(1); + + // Force a remount by changing the inner function's signature. + // Only the inner function's module re-runs; the wrapper and the + // element referencing it are not re-created. + await act(async () => { + await patch(() => { + function Inner({label}) { + innerRenders++; + return

{label}

; + } + $RefreshReg$(Inner, 'Inner'); + $RefreshSig$(Inner, 'sig2'); + return Inner; + }); + }); + expect(innerRenders).toBe(2); + const innerEl = container.firstChild.firstChild; + + // The remounted fiber must still be a memo: equal props stay + // blocked, and the fiber reconciles against the original element + // instead of being replaced again. + await act(async () => { + container.firstChild.click(); + }); + expect(innerRenders).toBe(2); + expect(container.firstChild.firstChild).toBe(innerEl); + } + }); + it('resets state when switching between different component types', async () => { if (__DEV__) { await act(async () => {