Skip to content

Commit

Permalink
Make LegacyHidden match semantics of old fork (#18998)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite authored May 26, 2020
1 parent 3ca1904 commit 03e6b8b
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,38 @@ describe('ReactDOMServerPartialHydration', () => {
SuspenseList = React.SuspenseList;
});

// Note: This is based on a similar component we use in www. We can delete
// once the unstable_LegacyHidden API exists in both forks, and once the
// extra div wrapper is no longer neccessary.
function LegacyHiddenDiv({children, mode}) {
let wrappedChildren;
if (gate(flags => flags.new)) {
// The new reconciler does not support `<div hidden={true} />`. The
// equivalent behavior was moved to a special type, unstable_LegacyHidden.
// Eventually, we will replace this with an official API.
wrappedChildren = (
<React.unstable_LegacyHidden
mode={mode === 'hidden' ? 'unstable-defer-without-hiding' : mode}>
{children}
</React.unstable_LegacyHidden>
);
} else {
// The old reconciler fork does not support the new type. Use the old
// `<div hidden={true} />` API. Once we remove this branch, we can also
// remove the extra DOM node wrapper around the children.
wrappedChildren = children;
}

return (
<div
hidden={
mode === 'hidden' ? 'unstable-do-not-use-legacy-hidden' : undefined
}>
{wrappedChildren}
</div>
);
}

// @gate experimental
it('hydrates a parent even if a child Suspense boundary is blocked', async () => {
let suspend = false;
Expand Down Expand Up @@ -2810,18 +2842,21 @@ describe('ReactDOMServerPartialHydration', () => {
expect(ref.current).not.toBe(null);
});

// This test fails, in both forks. Without a boundary, the deferred tree won't
// re-enter hydration mode. It doesn't come up in practice because there's
// always a parent Suspense boundary. But it's still a bug. Leaving for a
// follow up.
//
// @gate FIXME
// @gate experimental
// @gate new
it('renders a hidden LegacyHidden component', async () => {
const LegacyHidden = React.unstable_LegacyHidden;

it('hydrates a hidden subtree outside of a Suspense boundary', async () => {
const ref = React.createRef();

function App() {
return (
<LegacyHidden mode="hidden">
<LegacyHiddenDiv mode="hidden">
<span ref={ref}>Hidden child</span>
</LegacyHidden>
</LegacyHiddenDiv>
);
}

Expand All @@ -2831,27 +2866,25 @@ describe('ReactDOMServerPartialHydration', () => {
container.innerHTML = finalHTML;

const span = container.getElementsByTagName('span')[0];
expect(span).toBe(undefined);
expect(span.innerHTML).toBe('Hidden child');

const root = ReactDOM.createRoot(container, {hydrate: true});
root.render(<App />);
Scheduler.unstable_flushAll();
expect(ref.current.innerHTML).toBe('Hidden child');
expect(ref.current).toBe(span);
expect(span.innerHTML).toBe('Hidden child');
});

// @gate experimental
// @gate new
it('renders a hidden LegacyHidden component inside a Suspense boundary', async () => {
const LegacyHidden = React.unstable_LegacyHidden;

const ref = React.createRef();

function App() {
return (
<Suspense fallback="Loading...">
<LegacyHidden mode="hidden">
<LegacyHiddenDiv mode="hidden">
<span ref={ref}>Hidden child</span>
</LegacyHidden>
</LegacyHiddenDiv>
</Suspense>
);
}
Expand All @@ -2862,26 +2895,24 @@ describe('ReactDOMServerPartialHydration', () => {
container.innerHTML = finalHTML;

const span = container.getElementsByTagName('span')[0];
expect(span).toBe(undefined);
expect(span.innerHTML).toBe('Hidden child');

const root = ReactDOM.createRoot(container, {hydrate: true});
root.render(<App />);
Scheduler.unstable_flushAll();
expect(ref.current.innerHTML).toBe('Hidden child');
expect(ref.current).toBe(span);
expect(span.innerHTML).toBe('Hidden child');
});

// @gate experimental
// @gate new
it('renders a visible LegacyHidden component', async () => {
const LegacyHidden = React.unstable_LegacyHidden;

const ref = React.createRef();

function App() {
return (
<LegacyHidden mode="visible">
<LegacyHiddenDiv mode="visible">
<span ref={ref}>Hidden child</span>
</LegacyHidden>
</LegacyHiddenDiv>
);
}

Expand Down
20 changes: 8 additions & 12 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1020,18 +1020,14 @@ class ReactDOMServerRenderer {
}

switch (elementType) {
case REACT_LEGACY_HIDDEN_TYPE: {
if (!enableSuspenseServerRenderer) {
break;
}
if (((nextChild: any): ReactElement).props.mode === 'hidden') {
// In hidden mode, render nothing.
return '';
}
// Otherwise the tree is visible, so act like a fragment.
}
// Intentional fall through
// eslint-disable-next-line no-fallthrough
// TODO: LegacyHidden acts the same as a fragment. This only works
// because we currently 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 would have to hide the
// initial HTML in some other way.
case REACT_LEGACY_HIDDEN_TYPE:
case REACT_DEBUG_TRACING_MODE_TYPE:
case REACT_STRICT_MODE_TYPE:
case REACT_PROFILER_TYPE:
Expand Down
9 changes: 1 addition & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ import {
reenterHydrationStateFromDehydratedSuspenseInstance,
resetHydrationState,
tryToClaimNextHydratableInstance,
getIsHydrating,
warnIfHydrating,
} from './ReactFiberHydrationContext.new';
import {
Expand Down Expand Up @@ -584,13 +583,7 @@ function updateOffscreenComponent(
};
workInProgress.memoizedState = nextState;
pushRenderLanes(workInProgress, renderLanes);
} else if (
!includesSomeLane(renderLanes, (OffscreenLane: Lane)) ||
// Server renderer does not render hidden subtrees, so if we're hydrating
// we should always bail out and schedule a subsequent render pass, to
// force a client render. Even if we're already at Offscreen priority.
(current === null && getIsHydrating())
) {
} else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
let nextBaseLanes;
if (prevState !== null) {
const prevBaseLanes = prevState.baseLanes;
Expand Down
3 changes: 3 additions & 0 deletions scripts/jest/TestFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const environmentFlags = {
experimental: __EXPERIMENTAL__,
// Similarly, should stable imply "classic"?
stable: !__EXPERIMENTAL__,

// Use this for tests that are known to be broken.
FIXME: false,
};

function getTestFlags() {
Expand Down

0 comments on commit 03e6b8b

Please sign in to comment.