From 3716c39d0ad364e2f4b6f38db106833bc5c6117e Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Nov 2022 15:16:33 +0000 Subject: [PATCH 1/7] Add ref cleanup function --- packages/react-dom/src/__tests__/refs-test.js | 135 ++++++++++++++---- .../react-reconciler/src/ReactFiber.new.js | 3 + .../react-reconciler/src/ReactFiber.old.js | 3 + .../src/ReactFiberCommitWork.new.js | 38 +++-- .../src/ReactFiberCommitWork.old.js | 38 +++-- .../src/ReactInternalTypes.js | 2 + 6 files changed, 172 insertions(+), 47 deletions(-) diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 14c8261d02468..fb3b4c47a5959 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -351,35 +351,6 @@ describe('ref swapping', () => { 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.', ); }); - - it('should warn about callback refs returning a function', () => { - const container = document.createElement('div'); - expect(() => { - ReactDOM.render(
() => {}} />, container); - }).toErrorDev('Unexpected return value from a callback ref in div'); - - // Cleanup should warn, too. - expect(() => { - ReactDOM.render(, container); - }).toErrorDev('Unexpected return value from a callback ref in div', { - withoutStack: true, - }); - - // No warning when returning non-functions. - ReactDOM.render(

({})} />, container); - ReactDOM.render(

null} />, container); - ReactDOM.render(

undefined} />, container); - - // Still warns on functions (not deduped). - expect(() => { - ReactDOM.render(

() => {}} />, container); - }).toErrorDev('Unexpected return value from a callback ref in div'); - expect(() => { - ReactDOM.unmountComponentAtNode(container); - }).toErrorDev('Unexpected return value from a callback ref in div', { - withoutStack: true, - }); - }); }); describe('root level refs', () => { @@ -534,3 +505,109 @@ describe('strings refs across renderers', () => { expect(inst.refs.child2).toBe(div2.firstChild); }); }); + +describe('refs return clean up function', () => { + it('calls clean up function if it exists', () => { + const container = document.createElement('div'); + let cleanUp = jest.fn(); + let setup = jest.fn(); + + ReactDOM.render( +
{ + setup(_ref); + return cleanUp; + }} + />, + container, + ); + + ReactDOM.render( +
{ + setup(_ref); + }} + />, + container, + ); + + expect(setup).toHaveBeenCalledTimes(2); + expect(cleanUp).toHaveBeenCalledTimes(1); + expect(cleanUp.mock.calls[0][0]).toBe(undefined); + + ReactDOM.render(
{}} />, container); + + expect(cleanUp).toHaveBeenCalledTimes(1); + expect(setup).toHaveBeenCalledTimes(3); + expect(setup.mock.calls[2][0]).toBe(null); + + cleanUp = jest.fn(); + setup = jest.fn(); + + ReactDOM.render( +
{ + setup(_ref); + return cleanUp; + }} + />, + container, + ); + + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanUp).toHaveBeenCalledTimes(0); + + ReactDOM.render( +
{ + setup(_ref); + return cleanUp; + }} + />, + container, + ); + + expect(setup).toHaveBeenCalledTimes(2); + expect(cleanUp).toHaveBeenCalledTimes(1); + }); + + it('handles ref functions with stable identity', () => { + const container = document.createElement('div'); + const cleanUp = jest.fn(); + const setup = jest.fn(); + + function _onRefChange(_ref) { + setup(_ref); + return cleanUp; + } + + ReactDOM.render( +
, + container, + ); + + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanUp).toHaveBeenCalledTimes(0); + + ReactDOM.render( +
, + container, + ); + + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanUp).toHaveBeenCalledTimes(0); + + ReactDOM.render( +
, + container, + ); + + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanUp).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 7b1c29543eba3..9a60de9797392 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -148,6 +148,7 @@ function FiberNode( this.index = 0; this.ref = null; + this.refCleanup = null; this.pendingProps = pendingProps; this.memoizedProps = null; @@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { workInProgress.sibling = current.sibling; workInProgress.index = current.index; workInProgress.ref = current.ref; + workInProgress.refCleanup = current.refCleanup; if (enableProfilerTimer) { workInProgress.selfBaseDuration = current.selfBaseDuration; @@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV( target.sibling = source.sibling; target.index = source.index; target.ref = source.ref; + target.refCleanup = source.refCleanup; target.pendingProps = source.pendingProps; target.memoizedProps = source.memoizedProps; target.updateQueue = source.updateQueue; diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index fd1468d4a5d7c..64297701cd472 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -148,6 +148,7 @@ function FiberNode( this.index = 0; this.ref = null; + this.refCleanup = null; this.pendingProps = pendingProps; this.memoizedProps = null; @@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { workInProgress.sibling = current.sibling; workInProgress.index = current.index; workInProgress.ref = current.ref; + workInProgress.refCleanup = current.refCleanup; if (enableProfilerTimer) { workInProgress.selfBaseDuration = current.selfBaseDuration; @@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV( target.sibling = source.sibling; target.index = source.index; target.ref = source.ref; + target.refCleanup = source.refCleanup; target.pendingProps = source.pendingProps; target.memoizedProps = source.memoizedProps; target.updateQueue = source.updateQueue; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 1a9b72c8aeb0b..ea45e6e390351 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -286,7 +286,32 @@ function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; - if (ref !== null) { + const refCleanup = current.refCleanup; + + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } + + if (refCleanup !== null) { + if (typeof ref === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + refCleanup(); + } finally { + recordLayoutEffectDuration(current); + } + } else { + refCleanup(); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } + } else if (ref !== null) { if (typeof ref === 'function') { let retVal; try { @@ -1583,14 +1608,9 @@ function commitAttachRef(finishedWork: Fiber) { } else { retVal = ref(instanceToUse); } - if (__DEV__) { - if (typeof retVal === 'function') { - console.error( - 'Unexpected return value from a callback ref in %s. ' + - 'A callback ref should not return a function.', - getComponentNameFromFiber(finishedWork), - ); - } + + if (typeof retVal === 'function') { + finishedWork.refCleanup = retVal; } } else { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index a873d23c3d729..35c9a9740eb0f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -286,7 +286,32 @@ function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; - if (ref !== null) { + const refCleanup = current.refCleanup; + + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } + + if (refCleanup !== null) { + if (typeof ref === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + refCleanup(); + } finally { + recordLayoutEffectDuration(current); + } + } else { + refCleanup(); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } + } else if (ref !== null) { if (typeof ref === 'function') { let retVal; try { @@ -1583,14 +1608,9 @@ function commitAttachRef(finishedWork: Fiber) { } else { retVal = ref(instanceToUse); } - if (__DEV__) { - if (typeof retVal === 'function') { - console.error( - 'Unexpected return value from a callback ref in %s. ' + - 'A callback ref should not return a function.', - getComponentNameFromFiber(finishedWork), - ); - } + + if (typeof retVal === 'function') { + finishedWork.refCleanup = retVal; } } else { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index a5f28fd5d2450..0b7a0c4f9ef1f 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -131,6 +131,8 @@ export type Fiber = { | (((handle: mixed) => void) & {_stringRef: ?string, ...}) | RefObject, + refCleanup: null | (() => void), + // Input is the data coming into process this fiber. Arguments. Props. pendingProps: any, // This type will be more specific once we overload the tag. memoizedProps: any, // The props used to create the output. From be50757d2d0a729177f864dd3151a03cd38adaa8 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Nov 2022 15:15:34 +0000 Subject: [PATCH 2/7] Delete commitDetachRef --- .../src/ReactFiberCommitWork.new.js | 22 ------------------- .../src/ReactFiberCommitWork.old.js | 22 ------------------- 2 files changed, 44 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index ea45e6e390351..e91326e5645a2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -1629,27 +1629,6 @@ function commitAttachRef(finishedWork: Fiber) { } } -function commitDetachRef(current: Fiber) { - const currentRef = current.ref; - if (currentRef !== null) { - if (typeof currentRef === 'function') { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - currentRef(null); - } finally { - recordLayoutEffectDuration(current); - } - } else { - currentRef(null); - } - } else { - // $FlowFixMe unable to narrow type to the non-function case - currentRef.current = null; - } - } -} - function detachFiberMutation(fiber: Fiber) { // Cut off the return pointer to disconnect it from the tree. // This enables us to detect and warn against state updates on an unmounted component. @@ -4470,7 +4449,6 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { export { commitPlacement, commitAttachRef, - commitDetachRef, invokeLayoutEffectMountInDEV, invokeLayoutEffectUnmountInDEV, invokePassiveEffectMountInDEV, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 35c9a9740eb0f..5bad2a9026f9f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -1629,27 +1629,6 @@ function commitAttachRef(finishedWork: Fiber) { } } -function commitDetachRef(current: Fiber) { - const currentRef = current.ref; - if (currentRef !== null) { - if (typeof currentRef === 'function') { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - currentRef(null); - } finally { - recordLayoutEffectDuration(current); - } - } else { - currentRef(null); - } - } else { - // $FlowFixMe unable to narrow type to the non-function case - currentRef.current = null; - } - } -} - function detachFiberMutation(fiber: Fiber) { // Cut off the return pointer to disconnect it from the tree. // This enables us to detect and warn against state updates on an unmounted component. @@ -4470,7 +4449,6 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { export { commitPlacement, commitAttachRef, - commitDetachRef, invokeLayoutEffectMountInDEV, invokeLayoutEffectUnmountInDEV, invokePassiveEffectMountInDEV, From 073cec2f465b1a346beee2e3aa222f706e7f9660 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Nov 2022 15:21:31 +0000 Subject: [PATCH 3/7] yarn prettier --- packages/react-dom/src/__tests__/refs-test.js | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index fb3b4c47a5959..4d0fb9b0d0e73 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -581,31 +581,20 @@ describe('refs return clean up function', () => { return cleanUp; } - ReactDOM.render( -
, - container, - ); + ReactDOM.render(
, container); expect(setup).toHaveBeenCalledTimes(1); expect(cleanUp).toHaveBeenCalledTimes(0); ReactDOM.render( -
, +
, container, ); expect(setup).toHaveBeenCalledTimes(1); expect(cleanUp).toHaveBeenCalledTimes(0); - ReactDOM.render( -
, - container, - ); + ReactDOM.render(
, container); expect(setup).toHaveBeenCalledTimes(1); expect(cleanUp).toHaveBeenCalledTimes(1); From b9a8ed756aab3bb30c67db6f5bb98e24a2cd5985 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 16 Nov 2022 15:46:47 +0000 Subject: [PATCH 4/7] Move cleanup to finally branch --- .../src/ReactFiberCommitWork.new.js | 13 +++++++------ .../src/ReactFiberCommitWork.old.js | 13 +++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index e91326e5645a2..9ede307f08a84 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -288,12 +288,6 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; - } - if (refCleanup !== null) { if (typeof ref === 'function') { try { @@ -309,6 +303,13 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { } } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } } } } else if (ref !== null) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 5bad2a9026f9f..6fc491bc7122e 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -288,12 +288,6 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; - } - if (refCleanup !== null) { if (typeof ref === 'function') { try { @@ -309,6 +303,13 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { } } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } } } } else if (ref !== null) { From 9d315ab328ab55299753d76c6eb48aad95606e50 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 17 Nov 2022 11:28:40 +0000 Subject: [PATCH 5/7] Remove typeof check --- .../src/ReactFiberCommitWork.new.js | 47 ++++++++----------- .../src/ReactFiberCommitWork.old.js | 47 ++++++++----------- 2 files changed, 40 insertions(+), 54 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 9ede307f08a84..2288fb40dbcda 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -288,28 +288,26 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - if (refCleanup !== null) { - if (typeof ref === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - refCleanup(); - } finally { - recordLayoutEffectDuration(current); - } - } else { + if (typeof refCleanup === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); refCleanup(); + } finally { + recordLayoutEffectDuration(current); } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } finally { - // `refCleanup` has been called. Nullify all references to it to prevent double invocation. - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; - } + } else { + refCleanup(); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; } } } else if (ref !== null) { @@ -1598,20 +1596,15 @@ function commitAttachRef(finishedWork: Fiber) { instanceToUse = instance; } if (typeof ref === 'function') { - let retVal; if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); - retVal = ref(instanceToUse); + finishedWork.refCleanup = ref(instanceToUse); } finally { recordLayoutEffectDuration(finishedWork); } } else { - retVal = ref(instanceToUse); - } - - if (typeof retVal === 'function') { - finishedWork.refCleanup = retVal; + finishedWork.refCleanup = ref(instanceToUse); } } else { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 6fc491bc7122e..a410c3501bf81 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -288,28 +288,26 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - if (refCleanup !== null) { - if (typeof ref === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - refCleanup(); - } finally { - recordLayoutEffectDuration(current); - } - } else { + if (typeof refCleanup === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); refCleanup(); + } finally { + recordLayoutEffectDuration(current); } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } finally { - // `refCleanup` has been called. Nullify all references to it to prevent double invocation. - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; - } + } else { + refCleanup(); + } + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; } } } else if (ref !== null) { @@ -1598,20 +1596,15 @@ function commitAttachRef(finishedWork: Fiber) { instanceToUse = instance; } if (typeof ref === 'function') { - let retVal; if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); - retVal = ref(instanceToUse); + finishedWork.refCleanup = ref(instanceToUse); } finally { recordLayoutEffectDuration(finishedWork); } } else { - retVal = ref(instanceToUse); - } - - if (typeof retVal === 'function') { - finishedWork.refCleanup = retVal; + finishedWork.refCleanup = ref(instanceToUse); } } else { if (__DEV__) { From d68d6e9a6873995a16aaf1ccc471837057016358 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 17 Nov 2022 13:19:20 +0000 Subject: [PATCH 6/7] Add unit test to cover conditionally returned clean up function --- packages/react-dom/src/__tests__/refs-test.js | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/react-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 4d0fb9b0d0e73..b03d49897eeda 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -599,4 +599,42 @@ describe('refs return clean up function', () => { expect(setup).toHaveBeenCalledTimes(1); expect(cleanUp).toHaveBeenCalledTimes(1); }); + + it('warns if clean up function is returned when called with null', () => { + const container = document.createElement('div'); + const cleanUp = jest.fn(); + const setup = jest.fn(); + let returnCleanUp = false; + + ReactDOM.render( +
{ + setup(_ref); + if (returnCleanUp) { + return cleanUp; + } + }} + />, + container, + ); + + expect(setup).toHaveBeenCalledTimes(1); + expect(cleanUp).toHaveBeenCalledTimes(0); + + returnCleanUp = true; + + expect(() => { + ReactDOM.render( +
{ + setup(_ref); + if (returnCleanUp) { + return cleanUp; + } + }} + />, + container, + ); + }).toErrorDev('Unexpected return value from a callback ref in div'); + }); }); From ebb8afc1f661a5a2df0b6197662ac99f82e617e7 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 17 Nov 2022 13:21:06 +0000 Subject: [PATCH 7/7] Remove null check --- .../src/ReactFiberCommitWork.new.js | 42 +++++++++---------- .../src/ReactFiberCommitWork.old.js | 42 +++++++++---------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 2288fb40dbcda..ae9337a01647f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -288,30 +288,30 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - if (typeof refCleanup === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); + if (ref !== null) { + if (typeof refCleanup === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + refCleanup(); + } finally { + recordLayoutEffectDuration(current); + } + } else { refCleanup(); - } finally { - recordLayoutEffectDuration(current); } - } else { - refCleanup(); - } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } finally { - // `refCleanup` has been called. Nullify all references to it to prevent double invocation. - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } } - } - } else if (ref !== null) { - if (typeof ref === 'function') { + } else if (typeof ref === 'function') { let retVal; try { if (shouldProfile(current)) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index a410c3501bf81..be4d5cfb8d5d9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -288,30 +288,30 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { const ref = current.ref; const refCleanup = current.refCleanup; - if (typeof refCleanup === 'function') { - try { - if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); + if (ref !== null) { + if (typeof refCleanup === 'function') { + try { + if (shouldProfile(current)) { + try { + startLayoutEffectTimer(); + refCleanup(); + } finally { + recordLayoutEffectDuration(current); + } + } else { refCleanup(); - } finally { - recordLayoutEffectDuration(current); } - } else { - refCleanup(); - } - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } finally { - // `refCleanup` has been called. Nullify all references to it to prevent double invocation. - current.refCleanup = null; - const finishedWork = current.alternate; - if (finishedWork != null) { - finishedWork.refCleanup = null; + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } finally { + // `refCleanup` has been called. Nullify all references to it to prevent double invocation. + current.refCleanup = null; + const finishedWork = current.alternate; + if (finishedWork != null) { + finishedWork.refCleanup = null; + } } - } - } else if (ref !== null) { - if (typeof ref === 'function') { + } else if (typeof ref === 'function') { let retVal; try { if (shouldProfile(current)) {