react icon indicating copy to clipboard operation
react copied to clipboard

Possibility to set min duration of Suspense fallback

Open tonix-tuft opened this issue 6 years ago • 31 comments

Do you want to request a feature or report a bug? Feature

What is the current behavior? I have played a bit with Concurrent Mode and the Suspense API. Really exiting features and I look forward to use them in a stable release. Thank you for everything you are doing!

Regarding the Suspense component, could it be nice to have a property (both in Concurrent Mode and in "normal/synchronous" mode) which would allow us to set the minimum duration of the Suspense fallback UI in case the fallback UI ever gets rendered?

What is the expected behavior? Let me do an example. Try clicking on the Next button in this codesandbox: https://codesandbox.io/s/cold-monad-ifr29.

You will see that the Suspense fallback UI is rendered and stays in the tree just for a little moment (~200ms) because both promises resolve in 1200ms, while useTransition has a timeoutMs of 1 second. In my opinion, this is a bit unpleasant to the eye.

Wouldn't it be nicer if we could tell the Suspense component something like "If you ever render the fallback, show it for at least N millisec."? E.g.:

...
function ProfilePage({ resource }) {
  return (
    <Suspense fallback={<h1>Loading profile...</h1>}
      // If the fallback ever gets rendered,
      // it will be shown for at least 1500 millisec.,
      // even if the promise resolves right after rendering the fallback.
      fallbackMinDurationMs={1500}>
      <ProfileDetails resource={resource} />
      <Suspense fallback={<h1>Loading posts...</h1>}>
        <ProfileTimeline resource={resource} />
      </Suspense>
    </Suspense>
  );
}
...

Consider an animated spinner used as a fallback of Suspense, if it happens that the promise resolves just a few milliseconds after rendering the fallback like above, the spinner will be rendered and suddenly disappear, without completing its animation cycle and showing an incomplete animation.

Whereas, if we could keep the spinner in the tree for at least fallbackMinDurationMs millisec. once rendered, we could improve its appearance in such cases.

The Suspense component responsible for rendering the fallback would have to wrap the caught Promise in a promise which would look something like this:

function maxDelayFallbackPromise({
  promise,
  timeoutMs, // ---> This would be the value of `useTransition`'s `timeoutMs`
  onFallback = () => {}, // ---> This code would run in case `timeoutMs` exceeds (i.e. when `Suspense`'s fallback UI is rendered)
  fallbackMinDurationMs
} = {}) {
  // Generate a unique identifier, like a string, a number, in order to identify which promise resolves first...
  const uniqueIdentifier = `promise_value_${Math.random()}`
  return Promise.race([
    promise,
    timeout(timeoutMs).then(() => uniqueIdentifier)
  ]).then(value => {
    if (value === uniqueIdentifier) {
      onFallback()
      return minDelayPromise(promise, fallbackMinDurationMs)
    }
    else {
      return value
    }
  })
}

Where timeout and minDelayPromise are:

function timeout(delayMs) {
  return new Promise(resolve => setTimeout(resolve, delayMs))
}

function minDelayPromise(promise, minDelay) {
  return Promise.all([
    promise,
    timeout(minDelay)
  ]).then(([value]) => {
    return value
  })
}

This could also apply to the isPending flag of useTransition...

Do you think such a feature could improve the UX in such cases?

UPDATE - 04/09/2022 - For anyone looking at this issue, there is a workaround to achieve this fallback min duration behaviour in React 17 🎉 , described here: https://github.com/facebook/react/issues/17351#issuecomment-1236303278

tonix-tuft avatar Nov 13 '19 00:11 tonix-tuft

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

stale[bot] avatar Feb 11 '20 03:02 stale[bot]

Ping to unmark as stale.

I think there's a few undocumented useTransition options that do what you want, but I'm not sure if we're gonna replace those with something else.

gaearon avatar Feb 11 '20 16:02 gaearon

@gaearon Thank you for the feedback, Dan. I look forward to having the possibility to use those options when the final Suspense API is finally released.

tonix-tuft avatar Feb 12 '20 07:02 tonix-tuft

Please don't close it, it's worth tracking.

gaearon avatar Feb 12 '20 12:02 gaearon

@gaearon OK :)

tonix-tuft avatar Feb 13 '20 04:02 tonix-tuft

We can manage that in promise fetch function I think this can do it

function delayPromise(promise, delay) {
  return new Promise((resolve, reject) => {
    let status = 'pending';
    let result;
    promise
      .then((_result) => {
        if (timeout) {
          result = _result;
          status = 'success';
          return;
        }

        resolve(_result);
      })
      .catch((_result) => {
        if (timeout) {
          result = _result;
          status = 'error';

          return;
        }

        reject(_result);
      });

    let timeout = setTimeout(() => {
      if (status === 'success') {
        resolve(result);
      } else if (status === 'error') {
        reject(result);
      }

      timeout = null;
    }, delay);
  });
}

and use that like this

let postsPromise = delayPromise(fetchPosts(), 1500);
const resource = wrapPromise(postsPromise);

hosseinmd avatar May 18 '20 06:05 hosseinmd

@hosseinmd Haven't tried it yet, but I don't think your code achieves the same behaviour.

You are starting the timeout concurrently as soon as you create the Promise responsible for fetching the data, whereas in my example the timeout gets created only as soon as the Suspense fallback UI is rendered, it won't be even be created if the promise resolves before the timeoutMs of useTransition has been reached.

tonix-tuft avatar May 18 '20 08:05 tonix-tuft

Yes, you are right. This code avoid hidden fallback until 1500 ms. timeout start from create promise resource. But you like to start that from UI rendered.

hosseinmd avatar May 18 '20 10:05 hosseinmd

Related to this feature request - what if the child component rendered with Suspense was given an additional prop representing the amount of time the fallback component was visible? That way the child component can own the logic for minimum fallback component duration time, such as:

const FALLBACK_MIN_DURATION_MS = 1500;
const ChildComponent = ({ fallbackVisibilityDuration }) => {
  const [showFallback, setShowFallback] = useState(false);
  useEffect(() => {
    if (fallbackVisibilityDuration < FALLBACK_MIN_DURATION_MS) {
      setShowFallback(true);
      setTimeout(() => { setShowFallback(false); }, (FALLBACK_MIN_DURATION_MS - fallbackVisibilityDuration));
    }
  });
  return showFallback ? <h1>Loading...</h1> : <h1>Foo</h1>;
}

PatNeedham avatar Aug 31 '20 02:08 PatNeedham

@PatNeedham This way ChildComponent has too many responsibilities from my point of view.

I think that a child component which suspends does not have to handle its fallback's min visibility duration, it's not its responsibility, that's the point of Suspense in the first place.

tonix-tuft avatar Aug 31 '20 06:08 tonix-tuft

I can imagine just setting internal state after fallback gets called and than after return just check if it was rendered for concrete amount of time... if yes render children of suspense...

damikun avatar Sep 16 '20 15:09 damikun

@damikun Could you provide an example? Thank you!

tonix-tuft avatar Sep 16 '20 16:09 tonix-tuft

@gaearon Hi Dan.

Any chance for this and other Suspense features to ever be released?

https://reactjs.org/docs/concurrent-mode-suspense.html https://www.youtube.com/watch?v=Tl0S7QkxFE4

Thank you!

tonix-tuft avatar Dec 09 '20 09:12 tonix-tuft

How is this going? In React17, throwing Promise during rendering becomes special , ErrorBoundary can't catch throwing Promise but anything else! This forbid us to implement our own version of Suspense with 'maxDuration' featrue.

hyf0 avatar Jan 22 '21 13:01 hyf0

@iheyunfei Even if we could catch that promise wrapped in an error (e.g. throw ({ promise: new Promise(...) });) that our custom Suspense implementing ErrorBoundary will catch, that will not help us because React unmounts the node that throws anything which is not a promise or handles a caught promise itself. I agree with you, it would be great to have more flexibility and catch promises in our custom components besides the builtin Suspense one.

tonix-tuft avatar Jan 24 '21 11:01 tonix-tuft

Just to note, we don't have a way to prevent this flash and probably won't for some time. The proper solution to this is probably exit animations but this is a big chunk of work we won't get to for a while.

gaearon avatar Mar 30 '22 00:03 gaearon

Hey guys! There are a few workarounds\hacks for the issue

The first one abuses Suspense behaviour when a component throws on every render. Also it has a small drawback compared to the second one.

const FunkyLoader = () => {
  throw new Promise((res) => {
    setTimeout(() => {
      res();
    }, 1200);
  });
};

const DelayedLoading = ({ delay }) => {
  const [ready, setReady] = useState(false);
  useEffect(() => {
    setTimeout(() => setReady(true), delay);
  }, []);
  return ready && <FunkyLoader />;
};

const SmartLoader = ({ children }) => (
  <Suspense fallback={<Loading debug="artifitial waiting" />}>
    <Suspense fallback={<DelayedLoading delay={500} />}>{children}</Suspense>
  </Suspense>
);

The second one seems more legit but https://github.com/facebook/react/issues/15069 makes it barely usable

class ErrorBoundary extends React.Component {
  state = { hasError: false };

  componentDidCatch(error) {
    error.current.then(() => this.setState({ hasError: false }));
  }

  static getDerivedStateFromError() {
    return { hasError: true };
  }

  render() {
    if (this.state.hasError) {
      return this.props.fallback;
    }

    return this.props.children;
  }
}


const DelayedLoading = ({ delay }) => {
  const [ready, setReady] = useState(false);
  useEffect(() => {
    setTimeout(() => setReady(true), delay);
  }, []);

  if (ready) {
    const promise = {
      current: new Promise((res) => setTimeout(res, 1200))
    };
    throw promise;
  } 
  return ready && null;
};

const SmartLoader = ({ children }) => (
  <ErrorBoundary fallback={<Loading debug="artifitial waiting" />}>
    <Suspense fallback={<DelayedLoading delay={500} />}>{children}</Suspense>
  </ErrorBoundary>
);

ZigGreen avatar Sep 01 '22 12:09 ZigGreen

@ZigGreen I like the idea behind the DelayedLoading fallback component and the FunkyLoader in the first example.

So does it work like this when you render SmartLoader ?

  1. children are rendered and some code suspends rendering by throwing a Promise
  2. Promise thrown at point 1 is caught by <Suspense fallback={<DelayedLoading delay={500} />}>, nothing is rendered yet
  3. After 500ms, DelayedLoading renders FunkyLoader which suspends by throwing another promise
  4. The promise thrown by FunkyLoader is caught by the outer <Suspense fallback={<Loading debug="artificial waiting" />}> block which renders its fallback right away (after those 500ms already elapsed).
  5. After another timeout of 1200ms, the FunkyLoader promise resolves and the component is re-rendered
  6. At this point, if the promise originally thrown by children at point 1 has been resolved, then children will be rendered, otherwise, what will happen? Will DelayedLoading start another 500ms timer until the children's promise resolves?

tonix-tuft avatar Sep 01 '22 21:09 tonix-tuft

Hi @ZigGreen and hi everyone!

So, @ZigGreen, I tried your code based on your first example, but the fallback min duration logic doesn't seem to work:

https://codesandbox.io/s/react-17-suspense-with-fallbackmindurationms-attempt-yn3ige?file=/src/index.js

Try clicking on the Next button, you will see that the fallback min delay works well (the fallback is rendered only after 1000ms if the promises thrown by the underlying components that suspended rendering have not resolved yet by the time 1000ms elapsed).

As for the fallback min duration, as soon as the fallback is rendered, even though the FunkyLoader (in the Codesandbox it's called PromiseThrower) throws a promise and sets a timeout, React 17 ignores that and attempts to re-render ProfileDetails and ProfileTimeline as soon as their promises resolve.

It basically works like this:

  1. React attempts to render <ProfileDetails /> and <ProfileTimeline />. Both components suspend;

  2. The <DelayedFallback /> (same as your <DelayedLoading />) fallback component gets rendered and sets a timeout of 1000ms (1 second);

  3. After 1 second, <ProfileDetails /> and <ProfileTimeline /> didn't resolve yet, so the <DelayedFallback /> re-renders thanks to its internal ready state and renders a <PromiseThrower /> which always throws a promise;

  4. This last <PromiseThrower /> promise is caught by the outermost <Suspense /> which renders the real fallback (Loading profile...);

  5. Now, when <ProfileDetails />'s and <ProfileTimeline />'s promises resolve, React ignores the promise thrown by <PromiseThrower /> and attempts to re-render <ProfileDetails /> and <ProfileTimeline /> right away without waiting for fallbackMinDurationMs to complete, which is not the behaviour I was looking for as I wanted those components to re-render only after the fallbackMinDurationMs timeout elapses once the fallback is rendered.

But... Your example was very very helpful 'cause I was finally able to implement the logic I wanted! 🎉🎉🎉

Have a look at this Codesandbox: https://codesandbox.io/s/react-17-smartsuspense-usesmartsuspendable-github-https-github-com-facebook-react-issues-17351-ss5keh?file=/src/index.js

Here, the fallback min duration logic is implemented through the React context API and a custom useSmartSuspendable hook which wraps the function that may suspend.

In this last Codesandbox, here is what happens:

  1. Same as above;

  2. Same as above;

  3. Same as above;

  4. Same as above;

  5. Now, when <ProfileDetails />'s and <ProfileTimeline />'s promises resolve, React attempts to re-render them, but thanks to the SmartSuspenseContext context and the custom useSmartSuspendable hook it will suspend one more time until the fallback min duration promise resolves (this promise is stored in a fallbackMinDurationPromiseRef ref once the real Loading profile... fallback is rendered and passed down the tree through the SmartSuspenseContext context);

  6. Thanks to this SmartSuspenseContext context and useSmartSuspendable hook, React will finally render <ProfileDetails /> and <ProfileTimeline /> only after 1500ms from when the Loading profile... fallback got rendered!

Cool! This allows you to e.g. keep a loading spinner in the DOM for at least 1500ms even though the underlying promises of suspended components have already resolved.

After almost 3 years, we finally have a workaround that allows us to achieve fine-grained Suspense loading in React 17.

Thanks to @ZigGreen and to everyone else who participated in this issue! 🙏

I guess React 18 has some builtin functionality for this.

@gaearon Do you think this issue can be closed now?

tonix-tuft avatar Sep 04 '22 10:09 tonix-tuft

I haven’t taken a close look but this solution feels pretty convoluted to me. It’s hard for me to say whether it has any nasty edge cases but it might. Until there’s a built-in solution (which probably won’t be available for a long time), if you really must show the fallback for a longer time, I would probably recommend doing it at the async function level by doing Promise.all with another Promise that “waits” for a specific duration (like 100ms).

Solutions that use context and Promises in refs to track Suspense state seem dangerously complicated to me. I wouldn’t use or recommend one.

gaearon avatar Sep 04 '22 13:09 gaearon

Yeap the first one works only over some old 16 version of react that probably had a bug. The bug allowed me to force react to wait for the promise thrown by FankyLoader. I've checked my solution over version 17 and no surprises it doesn't work anymore.

But! It came across my mind maybe there is a simpler solution... And well it seems there is one. Still feels a bit hacky to me tho


const DelayedLoading = ({ delay, onDelayEnd }) => {
  useEffect(() => {
    const id = setTimeout(() => onDelayEnd(true), delay);

    return () => clearTimeout(id);
  }, []);
  return null;
};

const SmartLoader = ({ children, wait = 500, freez = 2000 }) => {
  const [fakeLoading, setFakeLoading] = useState(false);
  const startFakeLoading = useCallback(() => {
    setFakeLoading(true);
    setTimeout(() => {
      setFakeLoading(false);
    }, freez);
  }, []);

  if (fakeLoading) {
    return <Loading debug="artifitial delay" />;
  }

  return (
    <Suspense
      fallback={<DelayedLoading delay={wait} onDelayEnd={startFakeLoading} />}
    >
      {children}
    </Suspense>
  );
};

Feel free to play with it https://codesandbox.io/s/react-suspense-delayed-loader-forked-hixit8?file=/src/index.js

ZigGreen avatar Sep 04 '22 13:09 ZigGreen

@ZigGreen Yep, this is a solution as well.

The only difference between this approach and the one I posted in my previous comment using React context and that custom useSmartSuspendable hook is that in your case, when the real loading fallback is rendered after 500ms, React will unmount the underlying <Suspense /> tree and therefore it will need to re-render the whole tree once the fallback min duration timeout (startFakeLoading timeout) is reached.

In the case of that custom React context and that useSmartSuspendable hook, React will only re-render those child components nested in the <Suspense /> tree, and never unmounts it.

tonix-tuft avatar Sep 05 '22 11:09 tonix-tuft

I haven’t taken a close look but this solution feels pretty convoluted to me. It’s hard for me to say whether it has any nasty edge cases but it might. Until there’s a built-in solution (which probably won’t be available for a long time), if you really must show the fallback for a longer time, I would probably recommend doing it at the async function level by doing Promise.all with another Promise that “waits” for a specific duration (like 100ms).

Solutions that use context and Promises in refs to track Suspense state seem dangerously complicated to me. I wouldn’t use or recommend one.

@gaearon I will play with it a little bit further and see if I can simplify the flow a bit, perhaps by throwing a wrapped Promise.all promise like you say.

I'll keep you posted.

tonix-tuft avatar Sep 05 '22 11:09 tonix-tuft

Yeah I see your point about re-rendering check this out. I tried to leave FunkyLoader but it keeps coming back :D

const FunkyLoader = () => {
  throw new Promise(() => {});
};

const Delayed = ({ delay, onDelayEnd, children }) => {
  const [showLoader, setShowLoader] = useState(false);
  useEffect(() => {
    const id = setTimeout(() => {
      onDelayEnd();
      setShowLoader(true);
    }, delay);

    return () => clearTimeout(id);
  }, []);
  return showLoader ? children : null;
};

const SmartLoader = ({ children, wait = 500, freez = 2000 }) => {
  const [fakeLoading, setFakeLoading] = useState(false);
  const startFakeLoading = useCallback(() => {
    setFakeLoading(true);
    setTimeout(() => {
      setFakeLoading(false);
    }, freez);
  }, []);

  const fallback = <Loading debug="artifitial delay" />;

  return (
    <Suspense fallback={fallback}>
      {fakeLoading && <FunkyLoader />}
      <Suspense
        fallback={
          <Delayed delay={wait} onDelayEnd={startFakeLoading}>
            {fallback}
          </Delayed>
        }
      >
        {children}
      </Suspense>
    </Suspense>
  );
};

The link stays the same https://codesandbox.io/s/react-suspense-delayed-loader-forked-hixit8?file=/src/index.js

ZigGreen avatar Sep 05 '22 12:09 ZigGreen

@ZigGreen Cool! This one doesn't even require relying on a custom hook, which is good.

I would say that the only thing to bear in mind here is that the fallback component could be rendered at two different times in two different places (i.e. <Suspense fallback={fallback}> and {fallback} as a child of <Delayed />).

If freez timeout is reached and the underlying promise hasn't been resolved yet (e.g. when the promise thrown by <Fast /> takes 4000ms to resolve), then the fallback of <Suspense fallback={fallback}> is unmounted and the same fallback is rendered here:

<Suspense
        fallback={
          <Delayed delay={wait} onDelayEnd={startFakeLoading}>
            {fallback}
          </Delayed>
        }
      >

It's not a big deal, but if you have an animated spinner, I guess that could reset its animation in such cases as a new fallback child of <Delayed /> spinner component will be rendered and replace the one rendered by the outermost <Suspense fallback={fallback} />.

What are your thoughts about that?

tonix-tuft avatar Sep 05 '22 15:09 tonix-tuft

No problemo

const DelayedLoader = ({ delay, onDelayEnd, onLoaded }) => {
  useEffect(() => {
    const id = setTimeout(onDelayEnd, delay);

    return () => {
      onLoaded();
      clearTimeout(id);
    }
  }, []);
  return null;
};

const SmartLoader = ({ children, wait = 500, freez = 2000 }) => {
  const [fakeLoading, setFakeLoading] = useState(false);
  const [realLoading, setRealLoading] = useState(false);
  const startFakeLoading = useCallback(() => {
    setFakeLoading(true);
    setRealLoading(true);
    setTimeout(() => {
      setFakeLoading(false);
    }, freez);
  }, []);
  const stopRealLoading = useCallback(() => {
      setRealLoading(false);
  }, []);

  return (
    <Suspense fallback={<Loading debug="artifitial delay" />}>
      {(fakeLoading || realLoading) && <FunkyLoader />}
      <Suspense
        fallback={
          <DelayedLoader
            onLoaded={stopRealLoading}
            delay={wait}
            onDelayEnd={startFakeLoading}
          />
        }
      >
        {children}
      </Suspense>
    </Suspense>
  );
};

new link for this time https://codesandbox.io/s/react-suspense-delayed-loader-forked-lbu5e0?file=/src/index.js

ZigGreen avatar Sep 05 '22 19:09 ZigGreen

No problemo

const DelayedLoader = ({ delay, onDelayEnd, onLoaded }) => {
  useEffect(() => {
    const id = setTimeout(onDelayEnd, delay);

    return () => {
      onLoaded();
      clearTimeout(id);
    }
  }, []);
  return null;
};

const SmartLoader = ({ children, wait = 500, freez = 2000 }) => {
  const [fakeLoading, setFakeLoading] = useState(false);
  const [realLoading, setRealLoading] = useState(false);
  const startFakeLoading = useCallback(() => {
    setFakeLoading(true);
    setRealLoading(true);
    setTimeout(() => {
      setFakeLoading(false);
    }, freez);
  }, []);
  const stopRealLoading = useCallback(() => {
      setRealLoading(false);
  }, []);

  return (
    <Suspense fallback={<Loading debug="artifitial delay" />}>
      {(fakeLoading || realLoading) && <FunkyLoader />}
      <Suspense
        fallback={
          <DelayedLoader
            onLoaded={stopRealLoading}
            delay={wait}
            onDelayEnd={startFakeLoading}
          />
        }
      >
        {children}
      </Suspense>
    </Suspense>
  );
};

new link for this time https://codesandbox.io/s/react-suspense-delayed-loader-forked-lbu5e0?file=/src/index.js

@ZigGreen I tried it, it doesn't work though, the loading fallback stays there indefinitely and <Fast /> is never rendered (both with React 16 and React 17).

I think the code can be simplified further: we can remove the outermost <Suspense /> and render the fallback only once if we move the <FunkyLoader /> rendering within the innermost <Suspense />, like this:

const SmartLoader = ({ children, wait = 500, freez = 2000 }) => {
  const [fakeLoading, setFakeLoading] = useState(false);
  const startFakeLoading = useCallback(() => {
    setFakeLoading(true);
    const timeoutId = setTimeout(() => {
      setFakeLoading(false);
    }, freez);
    return () => clearTimeout(timeoutId);
  }, [freez]);

  const fallback = <Loading debug="Artificial Delay..." />;

  return (
    <Suspense
      fallback={
        <Delayed delay={wait} onDelayEnd={startFakeLoading}>
          {fallback}
        </Delayed>
      }
    >
      {fakeLoading && <FunkyLoader />}
      {children}
    </Suspense>
  );
};

Seems to work pretty well 👍 , here is the Codesandbox:

https://codesandbox.io/s/react-suspense-delayed-loader-forked-it-works-pz0xwc?file=/src/index.js:895-1531

tonix-tuft avatar Sep 05 '22 20:09 tonix-tuft

very nice 🔥

ZigGreen avatar Sep 05 '22 20:09 ZigGreen

@ZigGreen I wouldn't have gotten there without your help 🙏 😃 👍

tonix-tuft avatar Sep 05 '22 20:09 tonix-tuft

@gaearon What about this?

https://codesandbox.io/s/react-17-smartsuspense-simplified-github-https-github-com-facebook-react-issues-17351-l26wor?file=/src/index.js

It's based on our attempts with @ZigGreen.

It renders a single <Suspense /> instead of two and doesn't rely on React context nor fancy custom hooks like in the previous examples.

It's pretty straightforward and inline with React principles.

tonix-tuft avatar Sep 05 '22 21:09 tonix-tuft