[Experiment] Warn if callback ref returns a function (#22313)
Dan Abramov committed
Sep 15, 2021 at 13:51 UTC
67222f044c582a1a10f3b9d15d9daf67d5dc5c4f
13 files changed
+103
-8
packages/react-dom/src/__tests__/refs-test.js
+30
@@ -351,6 +351,36 @@ describe('ref swapping', () => {
351
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
352
);
353
});
354
+
355
+ // @gate !__DEV__ || warnAboutCallbackRefReturningFunction
356
+ it('should warn about callback refs returning a function', () => {
357
+ const container = document.createElement('div');
358
+ expect(() => {
359
+ ReactDOM.render(<div ref={() => () => {}} />, container);
360
+ }).toErrorDev('Unexpected return value from a callback ref in div');
361
+
362
+ // Cleanup should warn, too.
363
+ expect(() => {
364
+ ReactDOM.render(<span />, container);
365
+ }).toErrorDev('Unexpected return value from a callback ref in div', {
366
+ withoutStack: true,
367
+ });
368
+
369
+ // No warning when returning non-functions.
370
+ ReactDOM.render(<p ref={() => ({})} />, container);
371
+ ReactDOM.render(<p ref={() => null} />, container);
372
+ ReactDOM.render(<p ref={() => undefined} />, container);
373
+
374
+ // Still warns on functions (not deduped).
375
+ expect(() => {
376
+ ReactDOM.render(<div ref={() => () => {}} />, container);
377
+ }).toErrorDev('Unexpected return value from a callback ref in div');
378
+ expect(() => {
379
+ ReactDOM.unmountComponentAtNode(container);
380
+ }).toErrorDev('Unexpected return value from a callback ref in div', {
381
+ withoutStack: true,
382
+ });
383
+ });
384
});
385
386
describe('root level refs', () => {
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+31
-4
@@ -37,6 +37,7 @@ import {
37
deletedTreeCleanUpLevel,
38
enableSuspenseLayoutEffectSemantics,
39
enableUpdaterTracking,
40
+ warnAboutCallbackRefReturningFunction,
41
} from 'shared/ReactFeatureFlags';
42
import {
43
FunctionComponent,
@@ -250,6 +251,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
251
const ref = current.ref;
252
if (ref !== null) {
253
if (typeof ref === 'function') {
254
+ let retVal;
255
try {
256
if (
257
enableProfilerTimer &&
@@ -258,17 +260,29 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
260
) {
261
try {
262
startLayoutEffectTimer();
261
- ref(null);
263
+ retVal = ref(null);
264
} finally {
265
recordLayoutEffectDuration(current);
266
}
267
} else {
266
- ref(null);
268
+ retVal = ref(null);
269
}
270
} catch (error) {
271
reportUncaughtErrorInDEV(error);
272
captureCommitPhaseError(current, nearestMountedAncestor, error);
273
}
274
+ if (__DEV__) {
275
+ if (
276
+ warnAboutCallbackRefReturningFunction &&
277
+ typeof retVal === 'function'
278
+ ) {
279
+ console.error(
280
+ 'Unexpected return value from a callback ref in %s. ' +
281
+ 'A callback ref should not return a function.',
282
+ getComponentNameFromFiber(current),
283
+ );
284
+ }
285
+ }
286
} else {
287
ref.current = null;
288
}
@@ -1077,6 +1091,7 @@ function commitAttachRef(finishedWork: Fiber) {
1091
instanceToUse = instance;
1092
}
1093
if (typeof ref === 'function') {
1094
+ let retVal;
1095
if (
1096
enableProfilerTimer &&
1097
enableProfilerCommitHooks &&
@@ -1084,12 +1099,24 @@ function commitAttachRef(finishedWork: Fiber) {
1099
) {
1100
try {
1101
startLayoutEffectTimer();
1087
- ref(instanceToUse);
1102
+ retVal = ref(instanceToUse);
1103
} finally {
1104
recordLayoutEffectDuration(finishedWork);
1105
}
1106
} else {
1092
- ref(instanceToUse);
1107
+ retVal = ref(instanceToUse);
1108
+ }
1109
+ if (__DEV__) {
1110
+ if (
1111
+ warnAboutCallbackRefReturningFunction &&
1112
+ typeof retVal === 'function'
1113
+ ) {
1114
+ console.error(
1115
+ 'Unexpected return value from a callback ref in %s. ' +
1116
+ 'A callback ref should not return a function.',
1117
+ getComponentNameFromFiber(finishedWork),
1118
+ );
1119
+ }
1120
}
1121
} else {
1122
if (__DEV__) {
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+31
-4
@@ -37,6 +37,7 @@ import {
37
deletedTreeCleanUpLevel,
38
enableSuspenseLayoutEffectSemantics,
39
enableUpdaterTracking,
40
+ warnAboutCallbackRefReturningFunction,
41
} from 'shared/ReactFeatureFlags';
42
import {
43
FunctionComponent,
@@ -250,6 +251,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
251
const ref = current.ref;
252
if (ref !== null) {
253
if (typeof ref === 'function') {
254
+ let retVal;
255
try {
256
if (
257
enableProfilerTimer &&
@@ -258,17 +260,29 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
260
) {
261
try {
262
startLayoutEffectTimer();
261
- ref(null);
263
+ retVal = ref(null);
264
} finally {
265
recordLayoutEffectDuration(current);
266
}
267
} else {
266
- ref(null);
268
+ retVal = ref(null);
269
}
270
} catch (error) {
271
reportUncaughtErrorInDEV(error);
272
captureCommitPhaseError(current, nearestMountedAncestor, error);
273
}
274
+ if (__DEV__) {
275
+ if (
276
+ warnAboutCallbackRefReturningFunction &&
277
+ typeof retVal === 'function'
278
+ ) {
279
+ console.error(
280
+ 'Unexpected return value from a callback ref in %s. ' +
281
+ 'A callback ref should not return a function.',
282
+ getComponentNameFromFiber(current),
283
+ );
284
+ }
285
+ }
286
} else {
287
ref.current = null;
288
}
@@ -1077,6 +1091,7 @@ function commitAttachRef(finishedWork: Fiber) {
1091
instanceToUse = instance;
1092
}
1093
if (typeof ref === 'function') {
1094
+ let retVal;
1095
if (
1096
enableProfilerTimer &&
1097
enableProfilerCommitHooks &&
@@ -1084,12 +1099,24 @@ function commitAttachRef(finishedWork: Fiber) {
1099
) {
1100
try {
1101
startLayoutEffectTimer();
1087
- ref(instanceToUse);
1102
+ retVal = ref(instanceToUse);
1103
} finally {
1104
recordLayoutEffectDuration(finishedWork);
1105
}
1106
} else {
1092
- ref(instanceToUse);
1107
+ retVal = ref(instanceToUse);
1108
+ }
1109
+ if (__DEV__) {
1110
+ if (
1111
+ warnAboutCallbackRefReturningFunction &&
1112
+ typeof retVal === 'function'
1113
+ ) {
1114
+ console.error(
1115
+ 'Unexpected return value from a callback ref in %s. ' +
1116
+ 'A callback ref should not return a function.',
1117
+ getComponentNameFromFiber(finishedWork),
1118
+ );
1119
+ }
1120
}
1121
} else {
1122
if (__DEV__) {
packages/shared/ReactFeatureFlags.js
+2
@@ -166,6 +166,8 @@ export const deferRenderPhaseUpdateToNextBatch = false;
166
167
export const enableUseRefAccessWarning = false;
168
169
+export const warnAboutCallbackRefReturningFunction = false;
170
+
171
export const enableRecursiveCommitTraversal = false;
172
173
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -63,6 +63,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
63
export const enableStrictEffects = __DEV__;
64
export const createRootStrictEffectsByDefault = false;
65
export const enableUseRefAccessWarning = false;
66
+export const warnAboutCallbackRefReturningFunction = false;
67
68
export const enableRecursiveCommitTraversal = false;
69
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -54,6 +54,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
54
export const enableStrictEffects = false;
55
export const createRootStrictEffectsByDefault = false;
56
export const enableUseRefAccessWarning = false;
57
+export const warnAboutCallbackRefReturningFunction = false;
58
59
export const enableRecursiveCommitTraversal = false;
60
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -54,6 +54,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
54
export const enableStrictEffects = false;
55
export const createRootStrictEffectsByDefault = false;
56
export const enableUseRefAccessWarning = false;
57
+export const warnAboutCallbackRefReturningFunction = false;
58
59
export const enableRecursiveCommitTraversal = false;
60
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
@@ -53,6 +53,7 @@ export const warnOnSubscriptionInsideStartTransition = false;
53
export const enableStrictEffects = false;
54
export const createRootStrictEffectsByDefault = false;
55
export const enableUseRefAccessWarning = false;
56
+export const warnAboutCallbackRefReturningFunction = false;
57
58
export const enableRecursiveCommitTraversal = false;
59
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -54,6 +54,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
54
export const enableStrictEffects = true;
55
export const createRootStrictEffectsByDefault = false;
56
export const enableUseRefAccessWarning = false;
57
+export const warnAboutCallbackRefReturningFunction = false;
58
59
export const enableRecursiveCommitTraversal = false;
60
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -54,6 +54,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
54
export const enableStrictEffects = false;
55
export const createRootStrictEffectsByDefault = false;
56
export const enableUseRefAccessWarning = false;
57
+export const warnAboutCallbackRefReturningFunction = false;
58
59
export const enableRecursiveCommitTraversal = false;
60
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -54,6 +54,7 @@ export const deferRenderPhaseUpdateToNextBatch = false;
54
export const enableStrictEffects = false;
55
export const createRootStrictEffectsByDefault = false;
56
export const enableUseRefAccessWarning = false;
57
+export const warnAboutCallbackRefReturningFunction = false;
58
59
export const enableRecursiveCommitTraversal = false;
60
export const disableSchedulerTimeoutInWorkLoop = false;
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -19,6 +19,7 @@ export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
19
export const enableLegacyFBSupport = __VARIANT__;
20
export const skipUnmountedBoundaries = __VARIANT__;
21
export const enableUseRefAccessWarning = __VARIANT__;
22
+export const warnAboutCallbackRefReturningFunction = __VARIANT__;
23
export const deletedTreeCleanUpLevel = __VARIANT__ ? 3 : 1;
24
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
25
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -64,6 +64,7 @@ export const warnAboutDefaultPropsOnFunctionComponents = false;
64
export const enableGetInspectorDataForInstanceInProduction = false;
65
export const enableSuspenseServerRenderer = true;
66
export const enableSelectiveHydration = true;
67
+export const warnAboutCallbackRefReturningFunction = true;
68
69
export const enableLazyElements = true;
70
export const enableCache = true;