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

Suspense duplicating content on refresh #2508

Closed
ondratra opened this issue Apr 28, 2020 · 2 comments · Fixed by #2661
Closed

Suspense duplicating content on refresh #2508

ondratra opened this issue Apr 28, 2020 · 2 comments · Fixed by #2661
Labels
bug known issue The issue is known and may be left as-is.

Comments

@ondratra
Copy link

ondratra commented Apr 28, 2020

Suspense doesn't work as expected when throwing promises multiple times. In such case(s) content gets duplicated - it seems that the old component disconnects from its parent aand stays in the dom.

This is not happening with React's Suspense.

Reproduction

Example of the problem in Preact:
https://codesandbox.io/s/amazing-jennings-6b21f?file=/src/index.jsx

The same example in React: (elements don't get duplicated)
https://codesandbox.io/s/prod-shape-345d2?file=/src/App.jsx

Steps to reproduce

Throw a promise multiple times from a component inside of a Suspense.

import { Suspense, useState } from "preact/compat";
import { Component, render } from "preact";

export default class App extends Component {
  render(props, { results = [] }) {
    return (
      <Suspense fallback="loading ...">
        <MyWrapper />
      </Suspense>
    );
  }
}

if (typeof window !== "undefined") {
  render(<App />, document.getElementById("root"));
}

function MyWrapper() {
  const [refreshState, onRefreshStateChange] = useState(0);
  const forceRefresh = () => onRefreshStateChange(refreshState + 1);

  return <MyAsyncComponent forceRefresh={forceRefresh} />;
}

let uglyCache = null;
function MyAsyncComponent(props) {
  if (uglyCache) {
    const tmp = uglyCache;
    uglyCache = null;
    return tmp;
  }

  const asyncLoad = async () => {
    await artificialDelay();

    const result = (
      <div>
        My happy component
        <button onClick={props.forceRefresh}>click me</button>
      </div>
    );
    uglyCache = result;
  };

  throw asyncLoad();
}

async function artificialDelay(time) {
  return new Promise(resolve => {
    setTimeout(() => resolve(), time);
  });
}

Expected Behavior

Content will not be duplicated.

Actual Behavior

Content gets duplicated on each throw promise after the first one.

@ondratra
Copy link
Author

The issue might be related to #2504 and #2488.

@developit developit added bug known issue The issue is known and may be left as-is. labels May 14, 2020
@sventschui
Copy link
Member

Something goes crazy with unmounting/parking the old vnodes. When wrapping the <MyWrapper /> in a <div> it is at least unmounted while the fallback is rendered but the old DOM node is inserted again after the suspension clears. I'll try to dig into this the following days.

Btw. seems we have a test for this case but it doesn't work 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug known issue The issue is known and may be left as-is.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants