Disable <div hidden /> API in old fork, too (#18917)
The motivation for doing this is to make it impossible for additional uses of pre-rendering to sneak into www without going through the LegacyHidden abstraction. Since this feature was already disabled in the new fork, this brings the two closer to parity. The LegacyHidden abstraction itself still needs to opt into pre-rendering somehow, so rather than totally disabling the feature, I updated the `hidden` prop check to be obnoxiously specific. Before, you could set it to any truthy value; now, you must set it to the string "unstable-do-not-use-legacy-hidden". The node will still be hidden in the DOM, since any truthy value will cause the browser to apply a style of `display: none`. I will have to update the LegacyHidden component in www to use the obnoxious string prop. This doesn't block merge, though, since the behavior is gated by a dynamic flag. I will update the component before I enable the flag.
Andrew Clark committed
May 13, 2020 at 20:01 UTC
b4a1a4980c98c6d8a7ced428a1adc9e278fec430
15 files changed
+38
-55
packages/react-dom/src/__tests__/ReactUpdates-test.js
+6
-2
@@ -29,7 +29,9 @@ describe('ReactUpdates', () => {
29
function LegacyHiddenDiv({hidden, children, ...props}) {
30
if (gate(flags => flags.new)) {
31
return (
32
- <div hidden={hidden} {...props}>
32
+ <div
33
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
34
+ {...props}>
35
<React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
36
{children}
37
</React.unstable_LegacyHidden>
@@ -37,7 +39,9 @@ describe('ReactUpdates', () => {
39
);
40
} else {
41
return (
40
- <div hidden={hidden} {...props}>
42
+ <div
43
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
44
+ {...props}>
45
{children}
46
</div>
47
);
packages/react-dom/src/client/ReactDOMHostConfig.js
+9
-1
@@ -78,6 +78,7 @@ import {
78
enableModernEventSystem,
79
enableCreateEventHandleAPI,
80
enableScopeAPI,
81
+ disableHiddenPropDeprioritization,
82
} from 'shared/ReactFeatureFlags';
83
import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
84
import {TOP_BEFORE_BLUR, TOP_AFTER_BLUR} from '../events/DOMTopLevelEventTypes';
@@ -372,7 +373,14 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
373
}
374
375
export function shouldDeprioritizeSubtree(type: string, props: Props): boolean {
375
- return !!props.hidden;
376
+ if (disableHiddenPropDeprioritization) {
377
+ // This is obnoxiously specific so that nobody uses it, but we can still opt
378
+ // in via an infra-level userspace abstraction.
379
+ return props.hidden === 'unstable-do-not-use-legacy-hidden';
380
+ } else {
381
+ // Legacy behavior. Any truthy value works.
382
+ return !!props.hidden;
383
+ }
384
}
385
386
export function createTextInstance(
packages/react-reconciler/src/ReactFiberBeginWork.new.js
-17
@@ -75,7 +75,6 @@ import {
75
warnAboutDefaultPropsOnFunctionComponents,
76
enableScopeAPI,
77
enableBlocksAPI,
78
- warnAboutDOMHiddenAttribute,
78
} from 'shared/ReactFeatureFlags';
79
import invariant from 'shared/invariant';
80
import shallowEqual from 'shared/shallowEqual';
@@ -1124,22 +1123,6 @@ function updateHostComponent(
1123
}
1124
1125
markRef(current, workInProgress);
1127
-
1128
- if (__DEV__) {
1129
- if (
1130
- warnAboutDOMHiddenAttribute &&
1131
- (workInProgress.mode & ConcurrentMode) !== NoMode &&
1132
- nextProps.hasOwnProperty('hidden')
1133
- ) {
1134
- // This warning will not be user visible. Only exists so React Core team
1135
- // can find existing callers and migrate them to the new API.
1136
- console.error(
1137
- 'Detected use of DOM `hidden` attribute. Should migrate to new API. ' +
1138
- '(owner: React Core)',
1139
- );
1140
- }
1141
- }
1142
-
1126
reconcileChildren(current, workInProgress, nextChildren, renderLanes);
1127
return workInProgress.child;
1128
}
packages/react-reconciler/src/ReactFiberBeginWork.old.js
-16
@@ -69,7 +69,6 @@ import {
69
warnAboutDefaultPropsOnFunctionComponents,
70
enableScopeAPI,
71
enableBlocksAPI,
72
- warnAboutDOMHiddenAttribute,
72
} from 'shared/ReactFeatureFlags';
73
import invariant from 'shared/invariant';
74
import shallowEqual from 'shared/shallowEqual';
@@ -1100,21 +1099,6 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {
1099
1100
markRef(current, workInProgress);
1101
1103
- if (__DEV__) {
1104
- if (
1105
- warnAboutDOMHiddenAttribute &&
1106
- (workInProgress.mode & ConcurrentMode) !== NoMode &&
1107
- nextProps.hasOwnProperty('hidden')
1108
- ) {
1109
- // This warning will not be user visible. Only exists so React Core team
1110
- // can find existing callers and migrate them to the new API.
1111
- console.error(
1112
- 'Detected use of DOM `hidden` attribute. Should migrate to new API. ' +
1113
- '(owner: React Core)',
1114
- );
1115
- }
1116
- }
1117
-
1102
// Check the host config to see if the children are offscreen/hidden.
1103
if (
1104
workInProgress.mode & ConcurrentMode &&
packages/react-refresh/src/__tests__/ReactFresh-test.js
+6
-2
@@ -79,7 +79,9 @@ describe('ReactFresh', () => {
79
function LegacyHiddenDiv({hidden, children, ...props}) {
80
if (gate(flags => flags.new)) {
81
return (
82
- <div hidden={hidden} {...props}>
82
+ <div
83
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
84
+ {...props}>
85
<React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
86
{children}
87
</React.unstable_LegacyHidden>
@@ -87,7 +89,9 @@ describe('ReactFresh', () => {
89
);
90
} else {
91
return (
90
- <div hidden={hidden} {...props}>
92
+ <div
93
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
94
+ {...props}>
95
{children}
96
</div>
97
);
packages/react/src/__tests__/ReactDOMTracing-test.internal.js
+6
-2
@@ -59,7 +59,9 @@ function loadModules() {
59
function LegacyHiddenDiv({hidden, children, ...props}) {
60
if (gate(flags => flags.new)) {
61
return (
62
- <div hidden={hidden} {...props}>
62
+ <div
63
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
64
+ {...props}>
65
<React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
66
{children}
67
</React.unstable_LegacyHidden>
@@ -67,7 +69,9 @@ function LegacyHiddenDiv({hidden, children, ...props}) {
69
);
70
} else {
71
return (
70
- <div hidden={hidden} {...props}>
72
+ <div
73
+ hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
74
+ {...props}>
75
{children}
76
</div>
77
);
packages/shared/ReactFeatureFlags.js
+1
-1
@@ -140,4 +140,4 @@ export const enableLegacyFBSupport = false;
140
export const deferRenderPhaseUpdateToNextBatch = true;
141
142
// Flag used by www build so we can log occurrences of legacy hidden API
143
-export const warnAboutDOMHiddenAttribute = false;
143
+export const disableHiddenPropDeprioritization = true;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
-1
@@ -48,7 +48,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
48
49
export const enableNewReconciler = false;
50
export const deferRenderPhaseUpdateToNextBatch = true;
51
-export const warnAboutDOMHiddenAttribute = false;
51
+export const disableHiddenPropDeprioritization = true;
52
53
// Flow magic to verify the exports of this file match the original version.
54
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
-1
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
-export const warnAboutDOMHiddenAttribute = false;
50
+export const disableHiddenPropDeprioritization = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
-1
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
-export const warnAboutDOMHiddenAttribute = false;
50
+export const disableHiddenPropDeprioritization = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
-1
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
-export const warnAboutDOMHiddenAttribute = false;
50
+export const disableHiddenPropDeprioritization = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.testing.js
+1
-1
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
-export const warnAboutDOMHiddenAttribute = false;
50
+export const disableHiddenPropDeprioritization = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
-1
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
47
48
export const enableNewReconciler = false;
49
export const deferRenderPhaseUpdateToNextBatch = true;
50
-export const warnAboutDOMHiddenAttribute = false;
50
+export const disableHiddenPropDeprioritization = true;
51
52
// Flow magic to verify the exports of this file match the original version.
53
// eslint-disable-next-line no-unused-vars
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+3
-7
@@ -21,6 +21,9 @@ export const enableModernEventSystem = __VARIANT__;
21
export const enableLegacyFBSupport = __VARIANT__;
22
export const enableDebugTracing = !__VARIANT__;
23
24
+// Temporary flag, in case we need to re-enable this feature.
25
+export const disableHiddenPropDeprioritization = __VARIANT__;
26
+
27
// This only has an effect in the new reconciler. But also, the new reconciler
28
// is only enabled when __VARIANT__ is true. So this is set to the opposite of
29
// __VARIANT__ so that it's `false` when running against the new reconciler.
@@ -36,13 +39,6 @@ export const deferRenderPhaseUpdateToNextBatch = !__VARIANT__;
39
export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;
40
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
41
39
-// Do not add the corresponding warning to the warning filter! Only exists so we
40
-// can detect callers and migrate them to the new API. Should not visible to
41
-// anyone outside React Core team.
42
-//
43
-// Disabled in our tests, but we'll enable in www.
44
-export const warnAboutDOMHiddenAttribute = false;
45
-
42
// TODO: These flags are hard-coded to the default values used in open source.
43
// Update the tests so that they pass in either mode, then set these
44
// to __VARIANT__.
packages/shared/forks/ReactFeatureFlags.www.js
+1
-1
@@ -27,7 +27,7 @@ export const {
27
enableLegacyFBSupport,
28
enableDebugTracing,
29
deferRenderPhaseUpdateToNextBatch,
30
- warnAboutDOMHiddenAttribute,
30
+ disableHiddenPropDeprioritization,
31
} = dynamicFeatureFlags;
32
33
// On WWW, __EXPERIMENTAL__ is used for a new modern build.