Skip to content

Add findHostInstance_deprecated to the React Native Renderer #17224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

elicwhite
Copy link
Member

In ReactNative TouchableNativeFeedback clones its child passing through handlers that get attached to a host instance. It also dispatches commands like:

UIManager.dispatchViewManagerCommand(
      ReactNative.findNodeHandle(this),
      UIManager.getViewManagerConfig('RCTView').Commands.setPressed,
      [pressed],
    );

In order to get this off of UIManager.dispatchViewManagerCommand we need to use the new API which requires a direct ref to a host component. Unfortunately, since TouchableNativeFeedback uses cloneElement, it doesn't currently require its children to be host components so it can't attach a ref itself.

This PR adds a new internal and immediately deprecated API to the renderers that works the same as findNodeHandle but returns the component instance itself instead of the nativeTag on the instance. This API is only needed for TouchableNativeFeedback to maintain backwards compatibility as we migrate it to support Fabric.

We also have a new API coming that will replace TouchableNativeFeedback that doesn't use cloneElement. Once that is out and TouchableNativeFeedback is deleted we will be able to get rid of this API.

For Facebook employees, see https://fb.workplace.com/groups/rn.fabric/permalink/1617555541709199/

@@ -779,6 +779,81 @@ describe('ReactFabric', () => {
expect(touchStart2).toBeCalled();
});

it('findHostInstance_deprecated should warn if used to find a host component inside StrictMode', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are copy pasted from findNodeHandle's with different assertions

@sizebot
Copy link

sizebot commented Oct 30, 2019

Size changes (stable)

Details of bundled changes.

Comparing: 8eee0eb...755e46c

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.2% +0.1% 747.08 KB 748.6 KB 158.2 KB 158.32 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.1% 276.87 KB 277.41 KB 47.3 KB 47.37 KB RN_FB_PROD
ReactNativeRenderer-profiling.js +0.2% +0.1% 285.75 KB 286.29 KB 49.01 KB 49.07 KB RN_FB_PROFILING
ReactFabric-dev.js +0.2% +0.1% 752.4 KB 753.92 KB 158.97 KB 159.1 KB RN_FB_DEV
ReactFabric-prod.js 🔺+0.2% 🔺+0.1% 268.44 KB 268.97 KB 45.86 KB 45.92 KB RN_FB_PROD
ReactFabric-profiling.js +0.2% +0.1% 278.49 KB 279.03 KB 47.72 KB 47.78 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js +0.2% +0.1% 746.92 KB 748.44 KB 158.11 KB 158.24 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.1% 276.88 KB 277.42 KB 47.29 KB 47.35 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +0.2% +0.1% 285.76 KB 286.3 KB 49 KB 49.06 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.2% +0.1% 752.23 KB 753.75 KB 158.9 KB 159.02 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+0.2% 🔺+0.1% 268.43 KB 268.97 KB 45.84 KB 45.9 KB RN_OSS_PROD
ReactFabric-profiling.js +0.2% +0.1% 278.49 KB 279.03 KB 47.71 KB 47.77 KB RN_OSS_PROFILING

Generated by 🚫 dangerJS against 755e46c

@sizebot
Copy link

sizebot commented Oct 30, 2019

Size changes (experimental)

Details of bundled changes.

Comparing: 8eee0eb...755e46c

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactFabric-prod.js 🔺+0.2% 🔺+0.1% 268.45 KB 268.99 KB 45.87 KB 45.92 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.2% +0.1% 746.93 KB 748.45 KB 158.12 KB 158.25 KB RN_OSS_DEV
ReactFabric-profiling.js +0.2% +0.1% 278.5 KB 279.04 KB 47.73 KB 47.79 KB RN_FB_PROFILING
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.1% 276.89 KB 277.42 KB 47.31 KB 47.37 KB RN_FB_PROD
ReactNativeRenderer-profiling.js +0.2% +0.1% 285.77 KB 286.3 KB 49.02 KB 49.08 KB RN_FB_PROFILING
ReactFabric-dev.js +0.2% +0.1% 752.24 KB 753.76 KB 158.9 KB 159.03 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+0.2% 🔺+0.1% 268.45 KB 268.98 KB 45.85 KB 45.91 KB RN_OSS_PROD
ReactFabric-profiling.js +0.2% +0.1% 278.51 KB 279.04 KB 47.72 KB 47.78 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.2% +0.1% 752.41 KB 753.93 KB 158.98 KB 159.11 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+0.2% 🔺+0.1% 276.89 KB 277.43 KB 47.3 KB 47.36 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +0.2% +0.1% 285.78 KB 286.31 KB 49 KB 49.07 KB RN_OSS_PROFILING
ReactNativeRenderer-dev.js +0.2% +0.1% 747.09 KB 748.61 KB 158.2 KB 158.33 KB RN_FB_DEV

Generated by 🚫 dangerJS against 755e46c

@elicwhite elicwhite merged commit 515746c into facebook:master Oct 30, 2019
@elicwhite elicwhite deleted the findHostInstance branch October 30, 2019 20:29
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 30, 2019
elicwhite added a commit to elicwhite/react that referenced this pull request Oct 31, 2019
trueadm pushed a commit to trueadm/react that referenced this pull request Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants