Make LegacyHidden match semantics of old fork (#18998)
Facebook currently relies on being able to hydrate hidden HTML. So skipping those trees is a regression. We don't have a proper solution for this in the new API yet. So I'm reverting it to match the old behavior. Now the server renderer will treat LegacyHidden the same as a fragment, with no other special behavior. We can only get away with this because we assume that every instance of LegacyHidden is accompanied by a host component wrapper. In the hidden mode, the host component is given a `hidden` attribute, which ensures that the initial HTML is not visible. To support the use of LegacyHidden as a true fragment, without an extra DOM node, we will have to hide the initial HTML in some other way.
Andrew Clark committed
May 25, 2020 at 18:16 UTC
03e6b8ba2fb1c2820d2a5286701ff2b40590b438
4 files changed
+63
-40
packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+51
-20
@@ -87,6 +87,38 @@ describe('ReactDOMServerPartialHydration', () => {
87
SuspenseList = React.SuspenseList;
88
});
89
90
+ // Note: This is based on a similar component we use in www. We can delete
91
+ // once the unstable_LegacyHidden API exists in both forks, and once the
92
+ // extra div wrapper is no longer neccessary.
93
+ function LegacyHiddenDiv({children, mode}) {
94
+ let wrappedChildren;
95
+ if (gate(flags => flags.new)) {
96
+ // The new reconciler does not support `<div hidden={true} />`. The
97
+ // equivalent behavior was moved to a special type, unstable_LegacyHidden.
98
+ // Eventually, we will replace this with an official API.
99
+ wrappedChildren = (
100
+ <React.unstable_LegacyHidden
101
+ mode={mode === 'hidden' ? 'unstable-defer-without-hiding' : mode}>
102
+ {children}
103
+ </React.unstable_LegacyHidden>
104
+ );
105
+ } else {
106
+ // The old reconciler fork does not support the new type. Use the old
107
+ // `<div hidden={true} />` API. Once we remove this branch, we can also
108
+ // remove the extra DOM node wrapper around the children.
109
+ wrappedChildren = children;
110
+ }
111
+
112
+ return (
113
+ <div
114
+ hidden={
115
+ mode === 'hidden' ? 'unstable-do-not-use-legacy-hidden' : undefined
116
+ }>
117
+ {wrappedChildren}
118
+ </div>
119
+ );
120
+ }
121
+
122
// @gate experimental
123
it('hydrates a parent even if a child Suspense boundary is blocked', async () => {
124
let suspend = false;
@@ -2810,18 +2842,21 @@ describe('ReactDOMServerPartialHydration', () => {
2842
expect(ref.current).not.toBe(null);
2843
});
2844
2845
+ // This test fails, in both forks. Without a boundary, the deferred tree won't
2846
+ // re-enter hydration mode. It doesn't come up in practice because there's
2847
+ // always a parent Suspense boundary. But it's still a bug. Leaving for a
2848
+ // follow up.
2849
+ //
2850
+ // @gate FIXME
2851
// @gate experimental
2814
- // @gate new
2815
- it('renders a hidden LegacyHidden component', async () => {
2816
- const LegacyHidden = React.unstable_LegacyHidden;
2817
-
2852
+ it('hydrates a hidden subtree outside of a Suspense boundary', async () => {
2853
const ref = React.createRef();
2854
2855
function App() {
2856
return (
2822
- <LegacyHidden mode="hidden">
2857
+ <LegacyHiddenDiv mode="hidden">
2858
<span ref={ref}>Hidden child</span>
2824
- </LegacyHidden>
2859
+ </LegacyHiddenDiv>
2860
);
2861
}
2862
@@ -2831,27 +2866,25 @@ describe('ReactDOMServerPartialHydration', () => {
2866
container.innerHTML = finalHTML;
2867
2868
const span = container.getElementsByTagName('span')[0];
2834
- expect(span).toBe(undefined);
2869
+ expect(span.innerHTML).toBe('Hidden child');
2870
2871
const root = ReactDOM.createRoot(container, {hydrate: true});
2872
root.render(<App />);
2873
Scheduler.unstable_flushAll();
2839
- expect(ref.current.innerHTML).toBe('Hidden child');
2874
+ expect(ref.current).toBe(span);
2875
+ expect(span.innerHTML).toBe('Hidden child');
2876
});
2877
2878
// @gate experimental
2843
- // @gate new
2879
it('renders a hidden LegacyHidden component inside a Suspense boundary', async () => {
2845
- const LegacyHidden = React.unstable_LegacyHidden;
2846
-
2880
const ref = React.createRef();
2881
2882
function App() {
2883
return (
2884
<Suspense fallback="Loading...">
2852
- <LegacyHidden mode="hidden">
2885
+ <LegacyHiddenDiv mode="hidden">
2886
<span ref={ref}>Hidden child</span>
2854
- </LegacyHidden>
2887
+ </LegacyHiddenDiv>
2888
</Suspense>
2889
);
2890
}
@@ -2862,26 +2895,24 @@ describe('ReactDOMServerPartialHydration', () => {
2895
container.innerHTML = finalHTML;
2896
2897
const span = container.getElementsByTagName('span')[0];
2865
- expect(span).toBe(undefined);
2898
+ expect(span.innerHTML).toBe('Hidden child');
2899
2900
const root = ReactDOM.createRoot(container, {hydrate: true});
2901
root.render(<App />);
2902
Scheduler.unstable_flushAll();
2870
- expect(ref.current.innerHTML).toBe('Hidden child');
2903
+ expect(ref.current).toBe(span);
2904
+ expect(span.innerHTML).toBe('Hidden child');
2905
});
2906
2907
// @gate experimental
2874
- // @gate new
2908
it('renders a visible LegacyHidden component', async () => {
2876
- const LegacyHidden = React.unstable_LegacyHidden;
2877
-
2909
const ref = React.createRef();
2910
2911
function App() {
2912
return (
2882
- <LegacyHidden mode="visible">
2913
+ <LegacyHiddenDiv mode="visible">
2914
<span ref={ref}>Hidden child</span>
2884
- </LegacyHidden>
2915
+ </LegacyHiddenDiv>
2916
);
2917
}
2918
packages/react-dom/src/server/ReactPartialRenderer.js
+8
-12
@@ -1020,18 +1020,14 @@ class ReactDOMServerRenderer {
1020
}
1021
1022
switch (elementType) {
1023
- case REACT_LEGACY_HIDDEN_TYPE: {
1024
- if (!enableSuspenseServerRenderer) {
1025
- break;
1026
- }
1027
- if (((nextChild: any): ReactElement).props.mode === 'hidden') {
1028
- // In hidden mode, render nothing.
1029
- return '';
1030
- }
1031
- // Otherwise the tree is visible, so act like a fragment.
1032
- }
1033
- // Intentional fall through
1034
- // eslint-disable-next-line no-fallthrough
1023
+ // TODO: LegacyHidden acts the same as a fragment. This only works
1024
+ // because we currently assume that every instance of LegacyHidden is
1025
+ // accompanied by a host component wrapper. In the hidden mode, the host
1026
+ // component is given a `hidden` attribute, which ensures that the
1027
+ // initial HTML is not visible. To support the use of LegacyHidden as a
1028
+ // true fragment, without an extra DOM node, we would have to hide the
1029
+ // initial HTML in some other way.
1030
+ case REACT_LEGACY_HIDDEN_TYPE:
1031
case REACT_DEBUG_TRACING_MODE_TYPE:
1032
case REACT_STRICT_MODE_TYPE:
1033
case REACT_PROFILER_TYPE:
packages/react-reconciler/src/ReactFiberBeginWork.new.js
+1
-8
@@ -167,7 +167,6 @@ import {
167
reenterHydrationStateFromDehydratedSuspenseInstance,
168
resetHydrationState,
169
tryToClaimNextHydratableInstance,
170
- getIsHydrating,
170
warnIfHydrating,
171
} from './ReactFiberHydrationContext.new';
172
import {
@@ -584,13 +583,7 @@ function updateOffscreenComponent(
583
};
584
workInProgress.memoizedState = nextState;
585
pushRenderLanes(workInProgress, renderLanes);
587
- } else if (
588
- !includesSomeLane(renderLanes, (OffscreenLane: Lane)) ||
589
- // Server renderer does not render hidden subtrees, so if we're hydrating
590
- // we should always bail out and schedule a subsequent render pass, to
591
- // force a client render. Even if we're already at Offscreen priority.
592
- (current === null && getIsHydrating())
593
- ) {
586
+ } else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
587
let nextBaseLanes;
588
if (prevState !== null) {
589
const prevBaseLanes = prevState.baseLanes;
scripts/jest/TestFlags.js
+3
@@ -41,6 +41,9 @@ const environmentFlags = {
41
experimental: __EXPERIMENTAL__,
42
// Similarly, should stable imply "classic"?
43
stable: !__EXPERIMENTAL__,
44
+
45
+ // Use this for tests that are known to be broken.
46
+ FIXME: false,
47
};
48
49
function getTestFlags() {