Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter out dehydrated Suspense nodes #199

Merged
merged 1 commit into from
Apr 20, 2019
Merged

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Apr 20, 2019

Fixes #197.

This fix is not ideal. In particular, it makes dehydrated Suspense nodes appear hidden rather than empty. Conceptually, they're showing a server-rendered fallback, but we don't have React tree for it yet. Maybe it would be nice to show <Suspense> in the tree for them, but currently it's too broken so I opted to completely hide them until their hydration instead.

The reason we need this is because dehydrated Suspense nodes have funky behavior like this which I think violates some assumptions made by DevTools (e.g. we don't expect a Fiber to pretend it's a completely new one and disconnect its alternate). See #198 for a broader issue.

Unfortunately I can't easily write a test for this because partial hydration is only enabled behind a flag. Here's an app shell fixture I used to test it manually. It's based on tests from facebook/react#14884. Note you'll have to toggle enableSuspenseServerRenderer flag in the bundle for this to work:

/** @flow */

// This test harness mounts each test app as a separate root to test multi-root applications.

import React from 'react';
import ReactDOM from 'react-dom';
import ReactDOMServer from 'react-dom/server'
import DeeplyNestedComponents from './DeeplyNestedComponents';
import EditableProps from './EditableProps';
import ElementTypes from './ElementTypes';
import InspectableElements from './InspectableElements';
import InteractionTracing from './InteractionTracing';
import ToDoList from './ToDoList';
import Toggle from './Toggle';
import SuspenseTree from './SuspenseTree';

import './styles.css';

const containers = [];


function mountTestApp1() {
  const container = document.createElement('div');

  ((document.body: any): HTMLBodyElement).appendChild(container);

  containers.push(container);

    let suspend = false;
    let promise = new Promise(resolvePromise => {});
    let ref = React.createRef();

    function Child() {
      if (suspend) {
        throw promise;
      } else {
        return 'Hello';
      }
    }

    function App() {
      return (
        <div>
          <React.Suspense fallback="Loading...">
            <span ref={ref}>
              <Child />
            </span>
          </React.Suspense>
        </div>
      );
    }

    // We're going to simulate what Fizz will do during streaming rendering.

    suspend = true;
    let loadingHTML = ReactDOMServer.renderToString(<App />);
    suspend = false;
    let finalHTML = ReactDOMServer.renderToString(<App />);

    container.innerHTML = loadingHTML;
    containers.push(container);

    let suspenseNode = container.firstChild.firstChild;
    suspenseNode.data = '$?';

    function streamInContent() {
      let temp = document.createElement('div');
      temp.innerHTML = finalHTML;
      let finalSuspenseNode = temp.firstChild.firstChild;
      let fallbackContent = suspenseNode.nextSibling;
      let finalContent = finalSuspenseNode.nextSibling;
      suspenseNode.parentNode.replaceChild(finalContent, fallbackContent);
      suspenseNode.data = '$';
      if (suspenseNode._reactRetry) {
        suspenseNode._reactRetry();
      }
    }

    suspend = false;
    let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
    root.render(<App />);

    setTimeout(() => {
      streamInContent();  
    }, 1000)

    setTimeout(() => {
      root.render(<App />);
    }, 2000);

    
}

function mountTestApp2() {
  const container = document.createElement('div');

  ((document.body: any): HTMLBodyElement).appendChild(container);

  containers.push(container);
   let suspend = false;
    let promise = new Promise(resolvePromise => {});
    let ref = React.createRef();

    function Child() {
      if (suspend) {
        throw promise;
      } else {
        return 'Hello';
      }
    }

    function App() {
      return (
        <div>
          <React.Suspense fallback="Loading...">
            <span ref={ref}>
              <Child />
            </span>
          </React.Suspense>
        </div>
      );
    }

    suspend = true;
    let finalHTML = ReactDOMServer.renderToString(<App />);
    container.innerHTML = finalHTML;

    suspend = false;
    let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
    root.render(<App />);
}

function unmountTestApp() {
  // containers.forEach(container => unmountComponentAtNode(container));
}

mountTestApp1();
mountTestApp2();

// window.parent.mountTestApp = mountTestApp;
// window.parent.unmountTestApp = unmountTestApp;

Before

Note how primary child doesn't appear until we toggle it once, and how the second example has a duplicate Suspense (one of which is completely borked).

1

After

Suspense nodes only show up after they're hydrated, and they work fine.

2

Follow-ups

We might want to reevaluate this later and support dehydrated Suspense more comprehensively. But I wouldn't want to add this until we can at least run tests against it.

@bvaughn bvaughn merged commit 3697ed2 into bvaughn:master Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate Suspense node
2 participants