Skip to content

Commit

Permalink
Deprecate findDOMNode in StrictMode (facebook#13841)
Browse files Browse the repository at this point in the history 
* Deprecate findDOMNode in StrictMode

There are two scenarios. One is that we pass a component instance that is
already in strict mode or the node that we find is in strict mode if
an outer component renders into strict mode.

I use a separate method findHostInstanceWithWarning for this so that
a) I can pass the method name (findDOMNode/findNodeHandle).
b) Can ignore this warning in React Native mixins/NativeComponent that use this helper.

I don't want to expose the fiber to the renderers themselves.
  • Loading branch information
@sebmarkbage @jetoneza
sebmarkbage authored and jetoneza committed Jan 23, 2019
1 parent bf43844 commit 661931a
Show file tree
Hide file tree
Showing 9 changed files with 344 additions and 11 deletions.
68 changes: 67 additions & 1 deletion packages/react-dom/src/__tests__/findDOMNode-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
const React = require('react');
const ReactDOM = require('react-dom');
const ReactTestUtils = require('react-dom/test-utils');
const StrictMode = React.StrictMode;

describe('findDOMNode', () => {
it('findDOMNode should return null if passed null', () => {
Expand Down Expand Up @@ -94,7 +95,72 @@ describe('findDOMNode', () => {
return <div />;
}
}

expect(() => ReactTestUtils.renderIntoDocument(<Bar />)).not.toThrow();
});

it('findDOMNode should warn if used to find a host component inside StrictMode', () => {
let parent = undefined;
let child = undefined;

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

ReactTestUtils.renderIntoDocument(
<ContainsStrictModeChild ref={n => (parent = n)} />,
);

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

it('findDOMNode should warn if passed a component that is inside StrictMode', () => {
let parent = undefined;
let child = undefined;

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

ReactTestUtils.renderIntoDocument(
<StrictMode>
<IsInStrictMode ref={n => (parent = n)} />
</StrictMode>,
);

let match;
expect(() => (match = ReactDOM.findDOMNode(parent))).toWarnDev([
'Warning: findDOMNode is deprecated in StrictMode. ' +
'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference.' +
'\n' +
'\n in div (at **)' +
'\n in IsInStrictMode (at **)' +
'\n in StrictMode (at **)' +
'\n' +
'\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-find-node',
]);
expect(match).toBe(child);
});
});
7 changes: 6 additions & 1 deletion packages/react-dom/src/client/ReactDOM.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,12 @@ const ReactDOM: Object = {
if ((componentOrElement: any).nodeType === ELEMENT_NODE) {
return (componentOrElement: any);
}

if (__DEV__) {
return DOMRenderer.findHostInstanceWithWarning(
componentOrElement,
'findDOMNode',
);
}
return DOMRenderer.findHostInstance(componentOrElement);
},

Expand Down
13 changes: 12 additions & 1 deletion packages/react-native-renderer/src/ReactFabric.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import warningWithoutStack from 'shared/warningWithoutStack';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
const findHostInstance = ReactFabricRenderer.findHostInstance;
const findHostInstanceWithWarning =
ReactFabricRenderer.findHostInstanceWithWarning;

function findNodeHandle(componentOrHandle: any): ?number {
if (__DEV__) {
Expand Down Expand Up @@ -60,7 +62,16 @@ function findNodeHandle(componentOrHandle: any): ?number {
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
return componentOrHandle.canonical._nativeTag;
}
const hostInstance = findHostInstance(componentOrHandle);
let hostInstance;
if (__DEV__) {
hostInstance = findHostInstanceWithWarning(
componentOrHandle,
'findNodeHandle',
);
} else {
hostInstance = findHostInstance(componentOrHandle);
}

if (hostInstance == null) {
return hostInstance;
}
Expand Down
13 changes: 12 additions & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import warningWithoutStack from 'shared/warningWithoutStack';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
const findHostInstance = ReactNativeFiberRenderer.findHostInstance;
const findHostInstanceWithWarning =
ReactNativeFiberRenderer.findHostInstanceWithWarning;

function findNodeHandle(componentOrHandle: any): ?number {
if (__DEV__) {
Expand Down Expand Up @@ -63,7 +65,16 @@ function findNodeHandle(componentOrHandle: any): ?number {
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
return componentOrHandle.canonical._nativeTag;
}
const hostInstance = findHostInstance(componentOrHandle);
let hostInstance;
if (__DEV__) {
hostInstance = findHostInstanceWithWarning(
componentOrHandle,
'findNodeHandle',
);
} else {
hostInstance = findHostInstance(componentOrHandle);
}

if (hostInstance == null) {
return hostInstance;
}
Expand Down
77 changes: 77 additions & 0 deletions packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ let ReactFabric;
let createReactNativeComponentClass;
let UIManager;
let FabricUIManager;
let StrictMode;

jest.mock('shared/ReactFeatureFlags', () =>
require('shared/forks/ReactFeatureFlags.native-fabric-oss'),
Expand All @@ -25,6 +26,7 @@ describe('ReactFabric', () => {
jest.resetModules();

React = require('react');
StrictMode = React.StrictMode;
ReactFabric = require('react-native-renderer/fabric');
FabricUIManager = require('FabricUIManager');
UIManager = require('UIManager');
Expand Down Expand Up @@ -436,4 +438,79 @@ describe('ReactFabric', () => {
// This could change in the future.
expect(touchStart2).toBeCalled();
});

it('findNodeHandle 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>
);
}
}

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

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

it('findNodeHandle 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.findNodeHandle(parent))).toWarnDev([
'Warning: findNodeHandle is deprecated in StrictMode. ' +
'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference.' +
'\n' +
'\n in RCTView (at **)' +
'\n in IsInStrictMode (at **)' +
'\n in StrictMode (at **)' +
'\n' +
'\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-find-node',
]);
expect(match).toBe(child._nativeTag);
});
});
77 changes: 77 additions & 0 deletions packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

let React;
let StrictMode;
let ReactNative;
let createReactNativeComponentClass;
let UIManager;
Expand All @@ -20,6 +21,7 @@ describe('ReactNative', () => {
jest.resetModules();

React = require('react');
StrictMode = React.StrictMode;
ReactNative = require('react-native-renderer');
UIManager = require('UIManager');
createReactNativeComponentClass = require('ReactNativeViewConfigRegistry')
Expand Down Expand Up @@ -258,4 +260,79 @@ describe('ReactNative', () => {
11,
);
});

it('findNodeHandle 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.findNodeHandle(parent))).toWarnDev([
'Warning: findNodeHandle is deprecated in StrictMode. ' +
'findNodeHandle was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
'Instead, add a ref directly to the element you want to reference.' +
'\n' +
'\n in RCTView (at **)' +
'\n in StrictMode (at **)' +
'\n in ContainsStrictModeChild (at **)' +
'\n' +
'\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-find-node',
]);
expect(match).toBe(child._nativeTag);
});

it('findNodeHandle 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.findNodeHandle(parent))).toWarnDev([
'Warning: findNodeHandle is deprecated in StrictMode. ' +
'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' +
'Instead, add a ref directly to the element you want to reference.' +
'\n' +
'\n in RCTView (at **)' +
'\n in IsInStrictMode (at **)' +
'\n in StrictMode (at **)' +
'\n' +
'\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-find-node',
]);
expect(match).toBe(child._nativeTag);
});
});
6 changes: 6 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof component.id === 'number') {
return component;
}
if (__DEV__) {
return NoopRenderer.findHostInstanceWithWarning(
component,
'findInstance',
);
}
return NoopRenderer.findHostInstance(component);
},

Expand Down
Loading

0 comments on commit 661931a

Please sign in to comment.