Skip to content

Commit 4773fdf

Browse files
sebmarkbagesebmarkbage
authored
Deprecate findDOMNode in StrictMode (#13841)
* 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.
1 parent c9be16f commit 4773fdf

File tree

9 files changed

+344
-11
lines changed

9 files changed

+344
-11
lines changed

‎packages/react-dom/src/__tests__/findDOMNode-test.js‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
const React = require('react');
1313
const ReactDOM = require('react-dom');
1414
const ReactTestUtils = require('react-dom/test-utils');
15+
const StrictMode = React.StrictMode;
1516

1617
describe('findDOMNode', () => {
1718
it('findDOMNode should return null if passed null', () => {
@@ -94,7 +95,72 @@ describe('findDOMNode', () => {
9495
return <div />;
9596
}
9697
}
97-
9898
expect(() => ReactTestUtils.renderIntoDocument(<Bar />)).not.toThrow();
9999
});
100+
101+
it('findDOMNode should warn if used to find a host component inside StrictMode', () => {
102+
let parent = undefined;
103+
let child = undefined;
104+
105+
class ContainsStrictModeChild extends React.Component {
106+
render() {
107+
return (
108+
<StrictMode>
109+
<div ref={n => (child = n)} />
110+
</StrictMode>
111+
);
112+
}
113+
}
114+
115+
ReactTestUtils.renderIntoDocument(
116+
<ContainsStrictModeChild ref={n => (parent = n)} />,
117+
);
118+
119+
let match;
120+
expect(() => (match = ReactDOM.findDOMNode(parent))).toWarnDev([
121+
'Warning: findDOMNode is deprecated in StrictMode. ' +
122+
'findDOMNode was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
123+
'Instead, add a ref directly to the element you want to reference.' +
124+
'\n' +
125+
'\n in div (at **)' +
126+
'\n in StrictMode (at **)' +
127+
'\n in ContainsStrictModeChild (at **)' +
128+
'\n' +
129+
'\nLearn more about using refs safely here:' +
130+
'\nhttps://fb.me/react-strict-mode-find-node',
131+
]);
132+
expect(match).toBe(child);
133+
});
134+
135+
it('findDOMNode should warn if passed a component that is inside StrictMode', () => {
136+
let parent = undefined;
137+
let child = undefined;
138+
139+
class IsInStrictMode extends React.Component {
140+
render() {
141+
return <div ref={n => (child = n)} />;
142+
}
143+
}
144+
145+
ReactTestUtils.renderIntoDocument(
146+
<StrictMode>
147+
<IsInStrictMode ref={n => (parent = n)} />
148+
</StrictMode>,
149+
);
150+
151+
let match;
152+
expect(() => (match = ReactDOM.findDOMNode(parent))).toWarnDev([
153+
'Warning: findDOMNode is deprecated in StrictMode. ' +
154+
'findDOMNode was passed an instance of IsInStrictMode which is inside StrictMode. ' +
155+
'Instead, add a ref directly to the element you want to reference.' +
156+
'\n' +
157+
'\n in div (at **)' +
158+
'\n in IsInStrictMode (at **)' +
159+
'\n in StrictMode (at **)' +
160+
'\n' +
161+
'\nLearn more about using refs safely here:' +
162+
'\nhttps://fb.me/react-strict-mode-find-node',
163+
]);
164+
expect(match).toBe(child);
165+
});
100166
});

‎packages/react-dom/src/client/ReactDOM.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,12 @@ const ReactDOM: Object = {
614614
if ((componentOrElement: any).nodeType === ELEMENT_NODE) {
615615
return (componentOrElement: any);
616616
}
617-
617+
if (__DEV__) {
618+
return DOMRenderer.findHostInstanceWithWarning(
619+
componentOrElement,
620+
'findDOMNode',
621+
);
622+
}
618623
return DOMRenderer.findHostInstance(componentOrElement);
619624
},
620625

‎packages/react-native-renderer/src/ReactFabric.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import warningWithoutStack from 'shared/warningWithoutStack';
2929

3030
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
3131
const findHostInstance = ReactFabricRenderer.findHostInstance;
32+
const findHostInstanceWithWarning =
33+
ReactFabricRenderer.findHostInstanceWithWarning;
3234

3335
function findNodeHandle(componentOrHandle: any): ?number {
3436
if (__DEV__) {
@@ -60,7 +62,16 @@ function findNodeHandle(componentOrHandle: any): ?number {
6062
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
6163
return componentOrHandle.canonical._nativeTag;
6264
}
63-
const hostInstance = findHostInstance(componentOrHandle);
65+
let hostInstance;
66+
if (__DEV__) {
67+
hostInstance = findHostInstanceWithWarning(
68+
componentOrHandle,
69+
'findNodeHandle',
70+
);
71+
} else {
72+
hostInstance = findHostInstance(componentOrHandle);
73+
}
74+
6475
if (hostInstance == null) {
6576
return hostInstance;
6677
}

‎packages/react-native-renderer/src/ReactNativeRenderer.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import warningWithoutStack from 'shared/warningWithoutStack';
3232

3333
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
3434
const findHostInstance = ReactNativeFiberRenderer.findHostInstance;
35+
const findHostInstanceWithWarning =
36+
ReactNativeFiberRenderer.findHostInstanceWithWarning;
3537

3638
function findNodeHandle(componentOrHandle: any): ?number {
3739
if (__DEV__) {
@@ -63,7 +65,16 @@ function findNodeHandle(componentOrHandle: any): ?number {
6365
if (componentOrHandle.canonical && componentOrHandle.canonical._nativeTag) {
6466
return componentOrHandle.canonical._nativeTag;
6567
}
66-
const hostInstance = findHostInstance(componentOrHandle);
68+
let hostInstance;
69+
if (__DEV__) {
70+
hostInstance = findHostInstanceWithWarning(
71+
componentOrHandle,
72+
'findNodeHandle',
73+
);
74+
} else {
75+
hostInstance = findHostInstance(componentOrHandle);
76+
}
77+
6778
if (hostInstance == null) {
6879
return hostInstance;
6980
}

‎packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ let ReactFabric;
1515
let createReactNativeComponentClass;
1616
let UIManager;
1717
let FabricUIManager;
18+
let StrictMode;
1819

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

2728
React = require('react');
29+
StrictMode = React.StrictMode;
2830
ReactFabric = require('react-native-renderer/fabric');
2931
FabricUIManager = require('FabricUIManager');
3032
UIManager = require('UIManager');
@@ -436,4 +438,79 @@ describe('ReactFabric', () => {
436438
// This could change in the future.
437439
expect(touchStart2).toBeCalled();
438440
});
441+
442+
it('findNodeHandle should warn if used to find a host component inside StrictMode', () => {
443+
const View = createReactNativeComponentClass('RCTView', () => ({
444+
validAttributes: {foo: true},
445+
uiViewClassName: 'RCTView',
446+
}));
447+
448+
let parent = undefined;
449+
let child = undefined;
450+
451+
class ContainsStrictModeChild extends React.Component {
452+
render() {
453+
return (
454+
<StrictMode>
455+
<View ref={n => (child = n)} />
456+
</StrictMode>
457+
);
458+
}
459+
}
460+
461+
ReactFabric.render(<ContainsStrictModeChild ref={n => (parent = n)} />, 11);
462+
463+
let match;
464+
expect(() => (match = ReactFabric.findNodeHandle(parent))).toWarnDev([
465+
'Warning: findNodeHandle is deprecated in StrictMode. ' +
466+
'findNodeHandle was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
467+
'Instead, add a ref directly to the element you want to reference.' +
468+
'\n' +
469+
'\n in RCTView (at **)' +
470+
'\n in StrictMode (at **)' +
471+
'\n in ContainsStrictModeChild (at **)' +
472+
'\n' +
473+
'\nLearn more about using refs safely here:' +
474+
'\nhttps://fb.me/react-strict-mode-find-node',
475+
]);
476+
expect(match).toBe(child._nativeTag);
477+
});
478+
479+
it('findNodeHandle should warn if passed a component that is inside StrictMode', () => {
480+
const View = createReactNativeComponentClass('RCTView', () => ({
481+
validAttributes: {foo: true},
482+
uiViewClassName: 'RCTView',
483+
}));
484+
485+
let parent = undefined;
486+
let child = undefined;
487+
488+
class IsInStrictMode extends React.Component {
489+
render() {
490+
return <View ref={n => (child = n)} />;
491+
}
492+
}
493+
494+
ReactFabric.render(
495+
<StrictMode>
496+
<IsInStrictMode ref={n => (parent = n)} />
497+
</StrictMode>,
498+
11,
499+
);
500+
501+
let match;
502+
expect(() => (match = ReactFabric.findNodeHandle(parent))).toWarnDev([
503+
'Warning: findNodeHandle is deprecated in StrictMode. ' +
504+
'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' +
505+
'Instead, add a ref directly to the element you want to reference.' +
506+
'\n' +
507+
'\n in RCTView (at **)' +
508+
'\n in IsInStrictMode (at **)' +
509+
'\n in StrictMode (at **)' +
510+
'\n' +
511+
'\nLearn more about using refs safely here:' +
512+
'\nhttps://fb.me/react-strict-mode-find-node',
513+
]);
514+
expect(match).toBe(child._nativeTag);
515+
});
439516
});

‎packages/react-native-renderer/src/__tests__/ReactNativeMount-test.internal.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
'use strict';
1212

1313
let React;
14+
let StrictMode;
1415
let ReactNative;
1516
let createReactNativeComponentClass;
1617
let UIManager;
@@ -20,6 +21,7 @@ describe('ReactNative', () => {
2021
jest.resetModules();
2122

2223
React = require('react');
24+
StrictMode = React.StrictMode;
2325
ReactNative = require('react-native-renderer');
2426
UIManager = require('UIManager');
2527
createReactNativeComponentClass = require('ReactNativeViewConfigRegistry')
@@ -258,4 +260,79 @@ describe('ReactNative', () => {
258260
11,
259261
);
260262
});
263+
264+
it('findNodeHandle should warn if used to find a host component inside StrictMode', () => {
265+
const View = createReactNativeComponentClass('RCTView', () => ({
266+
validAttributes: {foo: true},
267+
uiViewClassName: 'RCTView',
268+
}));
269+
270+
let parent = undefined;
271+
let child = undefined;
272+
273+
class ContainsStrictModeChild extends React.Component {
274+
render() {
275+
return (
276+
<StrictMode>
277+
<View ref={n => (child = n)} />
278+
</StrictMode>
279+
);
280+
}
281+
}
282+
283+
ReactNative.render(<ContainsStrictModeChild ref={n => (parent = n)} />, 11);
284+
285+
let match;
286+
expect(() => (match = ReactNative.findNodeHandle(parent))).toWarnDev([
287+
'Warning: findNodeHandle is deprecated in StrictMode. ' +
288+
'findNodeHandle was passed an instance of ContainsStrictModeChild which renders StrictMode children. ' +
289+
'Instead, add a ref directly to the element you want to reference.' +
290+
'\n' +
291+
'\n in RCTView (at **)' +
292+
'\n in StrictMode (at **)' +
293+
'\n in ContainsStrictModeChild (at **)' +
294+
'\n' +
295+
'\nLearn more about using refs safely here:' +
296+
'\nhttps://fb.me/react-strict-mode-find-node',
297+
]);
298+
expect(match).toBe(child._nativeTag);
299+
});
300+
301+
it('findNodeHandle should warn if passed a component that is inside StrictMode', () => {
302+
const View = createReactNativeComponentClass('RCTView', () => ({
303+
validAttributes: {foo: true},
304+
uiViewClassName: 'RCTView',
305+
}));
306+
307+
let parent = undefined;
308+
let child = undefined;
309+
310+
class IsInStrictMode extends React.Component {
311+
render() {
312+
return <View ref={n => (child = n)} />;
313+
}
314+
}
315+
316+
ReactNative.render(
317+
<StrictMode>
318+
<IsInStrictMode ref={n => (parent = n)} />
319+
</StrictMode>,
320+
11,
321+
);
322+
323+
let match;
324+
expect(() => (match = ReactNative.findNodeHandle(parent))).toWarnDev([
325+
'Warning: findNodeHandle is deprecated in StrictMode. ' +
326+
'findNodeHandle was passed an instance of IsInStrictMode which is inside StrictMode. ' +
327+
'Instead, add a ref directly to the element you want to reference.' +
328+
'\n' +
329+
'\n in RCTView (at **)' +
330+
'\n in IsInStrictMode (at **)' +
331+
'\n in StrictMode (at **)' +
332+
'\n' +
333+
'\nLearn more about using refs safely here:' +
334+
'\nhttps://fb.me/react-strict-mode-find-node',
335+
]);
336+
expect(match).toBe(child._nativeTag);
337+
});
261338
});

‎packages/react-noop-renderer/src/createReactNoop.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
522522
if (typeof component.id === 'number') {
523523
return component;
524524
}
525+
if (__DEV__) {
526+
return NoopRenderer.findHostInstanceWithWarning(
527+
component,
528+
'findInstance',
529+
);
530+
}
525531
return NoopRenderer.findHostInstance(component);
526532
},
527533

0 commit comments

Comments
 (0)