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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @flow
*/

import type {ReactFabricType} from './ReactNativeTypes';
import type {ReactFabricType, HostComponent} from './ReactNativeTypes';
import type {ReactNodeList} from 'shared/ReactTypes';

import './ReactFabricInjection';
Expand Down Expand Up @@ -44,6 +44,54 @@ import warningWithoutStack from 'shared/warningWithoutStack';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

function findHostInstance_deprecated(
componentOrHandle: any,
): ?HostComponent<mixed> {
if (__DEV__) {
const owner = ReactCurrentOwner.current;
if (owner !== null && owner.stateNode !== null) {
warningWithoutStack(
owner.stateNode._warnedAboutRefsInRender,
'%s is accessing findNodeHandle inside its render(). ' +
'render() should be a pure function of props and state. It should ' +
'never access something that requires stale data from the previous ' +
'render, such as refs. Move this logic to componentDidMount and ' +
'componentDidUpdate instead.',
getComponentName(owner.type) || 'A component',
);

owner.stateNode._warnedAboutRefsInRender = true;
}
}
if (componentOrHandle == null) {
return null;
}
if (componentOrHandle._nativeTag) {
return componentOrHandle;
}
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
return componentOrHandle.canonical;
}
let hostInstance;
if (__DEV__) {
hostInstance = findHostInstanceWithWarning(
componentOrHandle,
'findHostInstance_deprecated',
);
} else {
hostInstance = findHostInstance(componentOrHandle);
}

if (hostInstance == null) {
return hostInstance;
}
if ((hostInstance: any).canonical) {
// Fabric
return (hostInstance: any).canonical;
}
return hostInstance;
}

function findNodeHandle(componentOrHandle: any): ?number {
if (__DEV__) {
const owner = ReactCurrentOwner.current;
Expand Down Expand Up @@ -108,6 +156,9 @@ const roots = new Map();
const ReactFabric: ReactFabricType = {
NativeComponent: ReactNativeComponent(findNodeHandle, findHostInstance),

// This is needed for implementation details of TouchableNativeFeedback
// Remove this once TouchableNativeFeedback doesn't use cloneElement
findHostInstance_deprecated,
findNodeHandle,

dispatchCommand(handle: any, command: string, args: Array<any>) {
Expand Down
53 changes: 52 additions & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @flow
*/

import type {ReactNativeType} from './ReactNativeTypes';
import type {ReactNativeType, HostComponent} from './ReactNativeTypes';
import type {ReactNodeList} from 'shared/ReactTypes';

import './ReactNativeInjection';
Expand Down Expand Up @@ -47,6 +47,54 @@ import warningWithoutStack from 'shared/warningWithoutStack';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;

function findHostInstance_deprecated(
componentOrHandle: any,
): ?HostComponent<mixed> {
if (__DEV__) {
const owner = ReactCurrentOwner.current;
if (owner !== null && owner.stateNode !== null) {
warningWithoutStack(
owner.stateNode._warnedAboutRefsInRender,
'%s is accessing findNodeHandle inside its render(). ' +
'render() should be a pure function of props and state. It should ' +
'never access something that requires stale data from the previous ' +
'render, such as refs. Move this logic to componentDidMount and ' +
'componentDidUpdate instead.',
getComponentName(owner.type) || 'A component',
);

owner.stateNode._warnedAboutRefsInRender = true;
}
}
if (componentOrHandle == null) {
return null;
}
if (componentOrHandle._nativeTag) {
return componentOrHandle;
}
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
return componentOrHandle.canonical;
}
let hostInstance;
if (__DEV__) {
hostInstance = findHostInstanceWithWarning(
componentOrHandle,
'findHostInstance_deprecated',
);
} else {
hostInstance = findHostInstance(componentOrHandle);
}

if (hostInstance == null) {
return hostInstance;
}
if ((hostInstance: any).canonical) {
// Fabric
return (hostInstance: any).canonical;
}
return hostInstance;
}

function findNodeHandle(componentOrHandle: any): ?number {
if (__DEV__) {
const owner = ReactCurrentOwner.current;
Expand Down Expand Up @@ -117,6 +165,9 @@ const roots = new Map();
const ReactNativeRenderer: ReactNativeType = {
NativeComponent: ReactNativeComponent(findNodeHandle, findHostInstance),

// This is needed for implementation details of TouchableNativeFeedback
// Remove this once TouchableNativeFeedback doesn't use cloneElement
findHostInstance_deprecated,
findNodeHandle,

dispatchCommand(handle: any, command: string, args: Array<any>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

let parent = undefined;
let child = undefined;

class ContainsStrictModeChild extends React.Component {
render() {
return (
<StrictMode>
<View ref={n => (child = n)} />
</StrictMode>
);
}
}

ReactFabric.render(<ContainsStrictModeChild ref={n => (parent = n)} />, 11);

let match;
expect(
() => (match = ReactFabric.findHostInstance_deprecated(parent)),
).toWarnDev([
'Warning: findHostInstance_deprecated is deprecated in StrictMode. ' +
'findHostInstance_deprecated was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
'Instead, add a ref directly to the element you want to reference. ' +
'Learn more about using refs safely here: ' +
'https://fb.me/react-strict-mode-find-node' +
'\n in RCTView (at **)' +
'\n in StrictMode (at **)' +
'\n in ContainsStrictModeChild (at **)',
]);
expect(match).toBe(child);
});

it('findHostInstance_deprecated should warn if passed a component that is inside StrictMode', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

let parent = undefined;
let child = undefined;

class IsInStrictMode extends React.Component {
render() {
return <View ref={n => (child = n)} />;
}
}

ReactFabric.render(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
11,
);

let match;
expect(
() => (match = ReactFabric.findHostInstance_deprecated(parent)),
).toWarnDev([
'Warning: findHostInstance_deprecated is deprecated in StrictMode. ' +
'findHostInstance_deprecated was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference. ' +
'Learn more about using refs safely here: ' +
'https://fb.me/react-strict-mode-find-node' +
'\n in RCTView (at **)' +
'\n in IsInStrictMode (at **)' +
'\n in StrictMode (at **)',
]);
expect(match).toBe(child);
});

it('findNodeHandle should warn if used to find a host component inside StrictMode', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,26 @@ describe('created with ReactFabric called with ReactNative', () => {
.ReactNativeViewConfigRegistry.register;
});

it('find Fabric instances with the RN renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
}));

let ref = React.createRef();

class Component extends React.Component {
render() {
return <View title="foo" />;
}
}

ReactFabric.render(<Component ref={ref} />, 11);

let instance = ReactNative.findHostInstance_deprecated(ref.current);
expect(instance._nativeTag).toBe(2);
});

it('find Fabric nodes with the RN renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
Expand Down Expand Up @@ -94,6 +114,26 @@ describe('created with ReactNative called with ReactFabric', () => {
.ReactNativeViewConfigRegistry.register;
});

it('find Paper instances with the Fabric renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
uiViewClassName: 'RCTView',
}));

let ref = React.createRef();

class Component extends React.Component {
render() {
return <View title="foo" />;
}
}

ReactNative.render(<Component ref={ref} />, 11);

let instance = ReactFabric.findHostInstance_deprecated(ref.current);
expect(instance._nativeTag).toBe(3);
});

it('find Paper nodes with the Fabric renderer', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {title: true},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,81 @@ describe('ReactNative', () => {
);
});

it('findHostInstance_deprecated should warn if used to find a host component inside StrictMode', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

let parent = undefined;
let child = undefined;

class ContainsStrictModeChild extends React.Component {
render() {
return (
<StrictMode>
<View ref={n => (child = n)} />
</StrictMode>
);
}
}

ReactNative.render(<ContainsStrictModeChild ref={n => (parent = n)} />, 11);

let match;
expect(
() => (match = ReactNative.findHostInstance_deprecated(parent)),
).toWarnDev([
'Warning: findHostInstance_deprecated is deprecated in StrictMode. ' +
'findHostInstance_deprecated was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
'Instead, add a ref directly to the element you want to reference. ' +
'Learn more about using refs safely here: ' +
'https://fb.me/react-strict-mode-find-node' +
'\n in RCTView (at **)' +
'\n in StrictMode (at **)' +
'\n in ContainsStrictModeChild (at **)',
]);
expect(match).toBe(child);
});

it('findHostInstance_deprecated should warn if passed a component that is inside StrictMode', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
uiViewClassName: 'RCTView',
}));

let parent = undefined;
let child = undefined;

class IsInStrictMode extends React.Component {
render() {
return <View ref={n => (child = n)} />;
}
}

ReactNative.render(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
11,
);

let match;
expect(
() => (match = ReactNative.findHostInstance_deprecated(parent)),
).toWarnDev([
'Warning: findHostInstance_deprecated is deprecated in StrictMode. ' +
'findHostInstance_deprecated was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference. ' +
'Learn more about using refs safely here: ' +
'https://fb.me/react-strict-mode-find-node' +
'\n in RCTView (at **)' +
'\n in IsInStrictMode (at **)' +
'\n in StrictMode (at **)',
]);
expect(match).toBe(child);
});

it('findNodeHandle should warn if used to find a host component inside StrictMode', () => {
const View = createReactNativeComponentClass('RCTView', () => ({
validAttributes: {foo: true},
Expand Down