From 20595f59c9359d0ac8e86519dba2cd46e7ddb316 Mon Sep 17 00:00:00 2001 From: Timothy Yung Date: Mon, 26 Jul 2021 21:02:06 -0700 Subject: [PATCH] Fix `ReactFabricHostComponent` methods if detached (#21967) --- .../src/ReactFabricHostConfig.js | 40 ++-- .../__tests__/ReactFabric-test.internal.js | 175 ++++++++++++++++++ 2 files changed, 201 insertions(+), 14 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 39058e9ac5a31..ac7fb64d922cd 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -137,17 +137,23 @@ class ReactFabricHostComponent { } measure(callback: MeasureOnSuccessCallback) { - fabricMeasure( - this._internalInstanceHandle.stateNode.node, - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + const {stateNode} = this._internalInstanceHandle; + if (stateNode != null) { + fabricMeasure( + stateNode.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } } measureInWindow(callback: MeasureInWindowOnSuccessCallback) { - fabricMeasureInWindow( - this._internalInstanceHandle.stateNode.node, - mountSafeCallback_NOT_REALLY_SAFE(this, callback), - ); + const {stateNode} = this._internalInstanceHandle; + if (stateNode != null) { + fabricMeasureInWindow( + stateNode.node, + mountSafeCallback_NOT_REALLY_SAFE(this, callback), + ); + } } measureLayout( @@ -168,12 +174,18 @@ class ReactFabricHostComponent { return; } - fabricMeasureLayout( - this._internalInstanceHandle.stateNode.node, - relativeToNativeNode._internalInstanceHandle.stateNode.node, - mountSafeCallback_NOT_REALLY_SAFE(this, onFail), - mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), - ); + const toStateNode = this._internalInstanceHandle.stateNode; + const fromStateNode = + relativeToNativeNode._internalInstanceHandle.stateNode; + + if (toStateNode != null && fromStateNode != null) { + fabricMeasureLayout( + toStateNode.node, + fromStateNode.node, + mountSafeCallback_NOT_REALLY_SAFE(this, onFail), + mountSafeCallback_NOT_REALLY_SAFE(this, onSuccess), + ); + } } setNativeProps(nativeProps: Object) { diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 6b776c085fd40..ebe763af02d06 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -414,6 +414,37 @@ describe('ReactFabric', () => { expect(successCallback).toHaveBeenCalledWith(10, 10, 100, 100, 0, 0); }); + it('should no-op if calling measure on unmounted refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + nativeFabricUIManager.measure.mockClear(); + + let viewRef; + act(() => { + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + }); + const dangerouslyRetainedViewRef = viewRef; + act(() => { + ReactFabric.stopSurface(11); + }); + + expect(nativeFabricUIManager.measure).not.toBeCalled(); + const successCallback = jest.fn(); + dangerouslyRetainedViewRef.measure(successCallback); + expect(nativeFabricUIManager.measure).not.toBeCalled(); + expect(successCallback).not.toBeCalled(); + }); + it('should call FabricUIManager.measureInWindow on ref.measureInWindow', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, @@ -442,6 +473,37 @@ describe('ReactFabric', () => { expect(successCallback).toHaveBeenCalledWith(10, 10, 100, 100); }); + it('should no-op if calling measureInWindow on unmounted refs', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + nativeFabricUIManager.measureInWindow.mockClear(); + + let viewRef; + act(() => { + ReactFabric.render( + { + viewRef = ref; + }} + />, + 11, + ); + }); + const dangerouslyRetainedViewRef = viewRef; + act(() => { + ReactFabric.stopSurface(11); + }); + + expect(nativeFabricUIManager.measureInWindow).not.toBeCalled(); + const successCallback = jest.fn(); + dangerouslyRetainedViewRef.measureInWindow(successCallback); + expect(nativeFabricUIManager.measureInWindow).not.toBeCalled(); + expect(successCallback).not.toBeCalled(); + }); + it('should support ref in ref.measureLayout', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, @@ -480,6 +542,119 @@ describe('ReactFabric', () => { expect(successCallback).toHaveBeenCalledWith(1, 1, 100, 100); }); + it('should no-op if calling measureLayout on unmounted "from" ref', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + nativeFabricUIManager.measureLayout.mockClear(); + + let viewRef; + let otherRef; + act(() => { + ReactFabric.render( + + { + viewRef = ref; + }} + /> + { + otherRef = ref; + }} + /> + , + 11, + ); + }); + const dangerouslyRetainedOtherRef = otherRef; + act(() => { + ReactFabric.render( + + { + viewRef = ref; + }} + /> + {null} + , + 11, + ); + }); + + expect(nativeFabricUIManager.measureLayout).not.toBeCalled(); + const successCallback = jest.fn(); + const failureCallback = jest.fn(); + viewRef.measureLayout( + dangerouslyRetainedOtherRef, + successCallback, + failureCallback, + ); + expect(nativeFabricUIManager.measureLayout).not.toBeCalled(); + expect(successCallback).not.toBeCalled(); + expect(failureCallback).not.toBeCalled(); + }); + + it('should no-op if calling measureLayout on unmounted "to" ref', () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + nativeFabricUIManager.measureLayout.mockClear(); + + let viewRef; + let otherRef; + act(() => { + ReactFabric.render( + + { + viewRef = ref; + }} + /> + { + otherRef = ref; + }} + /> + , + 11, + ); + }); + const dangerouslyRetainedViewRef = viewRef; + act(() => { + ReactFabric.render( + + {null} + { + otherRef = ref; + }} + /> + , + 11, + ); + }); + + expect(nativeFabricUIManager.measureLayout).not.toBeCalled(); + const successCallback = jest.fn(); + const failureCallback = jest.fn(); + dangerouslyRetainedViewRef.measureLayout( + otherRef, + successCallback, + failureCallback, + ); + expect(nativeFabricUIManager.measureLayout).not.toBeCalled(); + expect(successCallback).not.toBeCalled(); + expect(failureCallback).not.toBeCalled(); + }); + it('returns the correct instance and calls it in the callback', () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true},