SimpleMemoComponent should warn if a ref is given (#14178)
Fixes #13964.
Sophie Alpert committed
Nov 9, 2018 at 15:29 UTC
1a6ab1e9b5d3ab2c7083d89c00836b4cb9eff306
2 files changed
+100
-63
packages/react-reconciler/src/ReactFiberBeginWork.js
+62
-55
@@ -250,6 +250,9 @@ function updateMemoComponent(
250
// to a SimpleMemoComponent to allow fast path updates.
251
workInProgress.tag = SimpleMemoComponent;
252
workInProgress.type = type;
253
+ if (__DEV__) {
254
+ validateFunctionComponentInDev(workInProgress, type);
255
+ }
256
return updateSimpleMemoComponent(
257
current,
258
workInProgress,
@@ -990,68 +993,72 @@ function mountIndeterminateComponent(
993
// Proceed under the assumption that this is a function component
994
workInProgress.tag = FunctionComponent;
995
value = finishHooks(Component, props, value, context);
996
+ reconcileChildren(null, workInProgress, value, renderExpirationTime);
997
if (__DEV__) {
994
- if (Component) {
995
- warningWithoutStack(
996
- !Component.childContextTypes,
997
- '%s(...): childContextTypes cannot be defined on a function component.',
998
- Component.displayName || Component.name || 'Component',
999
- );
1000
- }
1001
- if (workInProgress.ref !== null) {
1002
- let info = '';
1003
- const ownerName = ReactCurrentFiber.getCurrentFiberOwnerNameInDevOrNull();
1004
- if (ownerName) {
1005
- info += '\n\nCheck the render method of `' + ownerName + '`.';
1006
- }
998
+ validateFunctionComponentInDev(workInProgress, Component);
999
+ }
1000
+ return workInProgress.child;
1001
+ }
1002
+}
1003
1008
- let warningKey = ownerName || workInProgress._debugID || '';
1009
- const debugSource = workInProgress._debugSource;
1010
- if (debugSource) {
1011
- warningKey = debugSource.fileName + ':' + debugSource.lineNumber;
1012
- }
1013
- if (!didWarnAboutFunctionRefs[warningKey]) {
1014
- didWarnAboutFunctionRefs[warningKey] = true;
1015
- warning(
1016
- false,
1017
- 'Function components cannot be given refs. ' +
1018
- 'Attempts to access this ref will fail.%s',
1019
- info,
1020
- );
1021
- }
1022
- }
1004
+function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
1005
+ if (Component) {
1006
+ warningWithoutStack(
1007
+ !Component.childContextTypes,
1008
+ '%s(...): childContextTypes cannot be defined on a function component.',
1009
+ Component.displayName || Component.name || 'Component',
1010
+ );
1011
+ }
1012
+ if (workInProgress.ref !== null) {
1013
+ let info = '';
1014
+ const ownerName = ReactCurrentFiber.getCurrentFiberOwnerNameInDevOrNull();
1015
+ if (ownerName) {
1016
+ info += '\n\nCheck the render method of `' + ownerName + '`.';
1017
+ }
1018
+
1019
+ let warningKey = ownerName || workInProgress._debugID || '';
1020
+ const debugSource = workInProgress._debugSource;
1021
+ if (debugSource) {
1022
+ warningKey = debugSource.fileName + ':' + debugSource.lineNumber;
1023
+ }
1024
+ if (!didWarnAboutFunctionRefs[warningKey]) {
1025
+ didWarnAboutFunctionRefs[warningKey] = true;
1026
+ warning(
1027
+ false,
1028
+ 'Function components cannot be given refs. ' +
1029
+ 'Attempts to access this ref will fail.%s',
1030
+ info,
1031
+ );
1032
+ }
1033
+ }
1034
1024
- if (typeof Component.getDerivedStateFromProps === 'function') {
1025
- const componentName = getComponentName(Component) || 'Unknown';
1035
+ if (typeof Component.getDerivedStateFromProps === 'function') {
1036
+ const componentName = getComponentName(Component) || 'Unknown';
1037
1027
- if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) {
1028
- warningWithoutStack(
1029
- false,
1030
- '%s: Function components do not support getDerivedStateFromProps.',
1031
- componentName,
1032
- );
1033
- didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true;
1034
- }
1035
- }
1038
+ if (!didWarnAboutGetDerivedStateOnFunctionComponent[componentName]) {
1039
+ warningWithoutStack(
1040
+ false,
1041
+ '%s: Function components do not support getDerivedStateFromProps.',
1042
+ componentName,
1043
+ );
1044
+ didWarnAboutGetDerivedStateOnFunctionComponent[componentName] = true;
1045
+ }
1046
+ }
1047
1037
- if (
1038
- typeof Component.contextType === 'object' &&
1039
- Component.contextType !== null
1040
- ) {
1041
- const componentName = getComponentName(Component) || 'Unknown';
1048
+ if (
1049
+ typeof Component.contextType === 'object' &&
1050
+ Component.contextType !== null
1051
+ ) {
1052
+ const componentName = getComponentName(Component) || 'Unknown';
1053
1043
- if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) {
1044
- warningWithoutStack(
1045
- false,
1046
- '%s: Function components do not support contextType.',
1047
- componentName,
1048
- );
1049
- didWarnAboutContextTypeOnFunctionComponent[componentName] = true;
1050
- }
1051
- }
1054
+ if (!didWarnAboutContextTypeOnFunctionComponent[componentName]) {
1055
+ warningWithoutStack(
1056
+ false,
1057
+ '%s: Function components do not support contextType.',
1058
+ componentName,
1059
+ );
1060
+ didWarnAboutContextTypeOnFunctionComponent[componentName] = true;
1061
}
1053
- reconcileChildren(null, workInProgress, value, renderExpirationTime);
1054
- return workInProgress.child;
1062
}
1063
}
1064
packages/react-reconciler/src/__tests__/ReactMemo-test.internal.js
+38
-8
@@ -15,6 +15,7 @@
15
let React;
16
let ReactFeatureFlags;
17
let ReactNoop;
18
+let Suspense;
19
20
describe('memo', () => {
21
beforeEach(() => {
@@ -23,6 +24,7 @@ describe('memo', () => {
24
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
25
React = require('react');
26
ReactNoop = require('react-noop-renderer');
27
+ ({Suspense} = React);
28
});
29
30
function span(prop) {
@@ -38,6 +40,42 @@ describe('memo', () => {
40
return {default: result};
41
}
42
43
+ it('warns when giving a ref (simple)', async () => {
44
+ // This test lives outside sharedTests because the wrappers don't forward
45
+ // refs properly, and they end up affecting the current owner which is used
46
+ // by the warning (making the messages not line up).
47
+ function App() {
48
+ return null;
49
+ }
50
+ App = React.memo(App);
51
+ function Outer() {
52
+ return <App ref={() => {}} />;
53
+ }
54
+ ReactNoop.render(<Outer />);
55
+ expect(ReactNoop.flush).toWarnDev([
56
+ 'Warning: Function components cannot be given refs. Attempts to access ' +
57
+ 'this ref will fail.',
58
+ ]);
59
+ });
60
+
61
+ it('warns when giving a ref (complex)', async () => {
62
+ // defaultProps means this won't use SimpleMemoComponent (as of this writing)
63
+ // SimpleMemoComponent is unobservable tho, so we can't check :)
64
+ function App() {
65
+ return null;
66
+ }
67
+ App.defaultProps = {};
68
+ App = React.memo(App);
69
+ function Outer() {
70
+ return <App ref={() => {}} />;
71
+ }
72
+ ReactNoop.render(<Outer />);
73
+ expect(ReactNoop.flush).toWarnDev([
74
+ 'Warning: Function components cannot be given refs. Attempts to access ' +
75
+ 'this ref will fail.',
76
+ ]);
77
+ });
78
+
79
// Tests should run against both the lazy and non-lazy versions of `memo`.
80
// To make the tests work for both versions, we wrap the non-lazy version in
81
// a lazy function component.
@@ -56,8 +94,6 @@ describe('memo', () => {
94
function sharedTests(label, memo) {
95
describe(`${label}`, () => {
96
it('bails out on props equality', async () => {
59
- const {Suspense} = React;
60
-
97
function Counter({count}) {
98
return <Text text={count} />;
99
}
@@ -93,8 +129,6 @@ describe('memo', () => {
129
});
130
131
it("does not bail out if there's a context change", async () => {
96
- const {Suspense} = React;
97
-
132
const CountContext = React.createContext(0);
133
134
function readContext(Context) {
@@ -142,8 +176,6 @@ describe('memo', () => {
176
});
177
178
it('accepts custom comparison function', async () => {
145
- const {Suspense} = React;
146
-
179
function Counter({count}) {
180
return <Text text={count} />;
181
}
@@ -184,8 +216,6 @@ describe('memo', () => {
216
});
217
218
it('supports non-pure class components', async () => {
187
- const {Suspense} = React;
188
-
219
class CounterInner extends React.Component {
220
static defaultProps = {suffix: '!'};
221
render() {