router icon indicating copy to clipboard operation
router copied to clipboard

`beforeLoad` adds ~400ms delay on page reload

Open cschmatzler opened this issue 1 year ago • 5 comments
trafficstars

Describe the bug

When adding beforeLoad to a route (even an empty one), the router gets mounted and then displays a white page for approx. 400ms before the actual page is rendered.

Your Example Website or App

https://stackblitz.com/github/tanstack/router/tree/main/examples/react/quickstart-file-based

Steps to Reproduce the Bug or Issue

  1. Fork the file-based QuickStart example on StackBlitz.
  2. Add an beforeLoad: () => {} to any page - I chose the about page.
  3. Go to the page you changed and reload the page in the browser.

Expected behavior

Simply adding the beforeLoad option should not materially change load times.

Screenshots or Videos

https://github.com/user-attachments/assets/eaa57efa-7c2f-4fad-b64d-a9f10017e9cf

Platform

  • OS: macOS Sonoma
  • Browser: Arc
  • Version: 1.56.0

Additional context

No response

cschmatzler avatar Aug 23 '24 17:08 cschmatzler

I've been experiencing this too, however my code might be a little different. Rendering the actual app is delayed by almost 1 second.

const InnerApp = () => {
    const auth = useAuthStore();

    if (auth.loading) {
        return <div>Loading...</div>;
    }

    return (
        <ThemeProvider theme={lightTheme}>
            <CssBaseline />
            <RouterProvider router={router} context={{ auth }} />
            <Toaster />
        </ThemeProvider>
    );
};

export default () => {
    return (
        <StrictMode>
            <QueryClientProvider client={queryClient}>
                <InnerApp />
            </QueryClientProvider>
        </StrictMode>
    );
};

I also checked the performance tab in devtools and it looks like nothing is happening (the selected range is when nothing is rendered) image

mreinertssgr avatar Aug 26 '24 20:08 mreinertssgr

As a temporary fix, add pendingMinMs: 0.

0xd8d avatar Sep 01 '24 10:09 0xd8d

I just noticed it happening on all the lazy routes. React profiler shows: Image looking at the offending function: https://github.com/TanStack/router/blob/0e16429b0d8af8d65040c06af3b895f4e10b310a/packages/react-router/src/Match.tsx#L19 It does wait for pendingMinMs ms before loading the match. Is there a catch to just setting pendingMinMs: 0?

macr avatar Sep 09 '24 12:09 macr

I just saw this issue and added the temporary fix and my app feels way faster now... This needs a fix.

Scooter1337 avatar Sep 12 '24 12:09 Scooter1337

Also noticed that adding this makes the app a lot more snappy, especially since we perform a lot of middleware/guard checks in beforeLoad() which takes almost no time. The default pendingMinMs is 500 which is pretty long.

Can be disabled for the entire router by setting defaultPendingMinMs: 0,

melv-n avatar Sep 24 '24 14:09 melv-n

Same here, I wasted soo much time digging in this and in the end it was the devtools, after removing them and setting the defaultPendingMinMs: 0 it's much better now, but I feel like it needs a fix

alex-uxify avatar Nov 06 '24 16:11 alex-uxify

The presence of beforeLoad or loader, even if they are empty (we still await their functions, even if they don’t return a promise) make pendingMinMs “trigger” (if there is a pending component defined).

Even if we have pendingMinMs: 0, the loader flashes quickly.

To goal of this setting is to ensure that loads don’t flash - if we need to see them, we should see them “for a bit”. Because loader or beforeLoad are defined, we determine that we should see the pendingComponent. We don’t really make a difference between a loader that takes 1ms, or one that takes 200ms.

If you only have a beforeLoad or a loader that is sufficiently fast, my suggestion would be to set:

pendingComponent: undefined,
pendingMinMs: 0

for that route. Mostly, I think the defaults are fine because loaders will be used to load data, which can take a couple hundreds of milliseconds, so we’d want to really avoid flashing in the loader (which even happens with pendingMinMs: 0

@schiller-manuel FYI

TkDodo avatar Nov 23 '24 17:11 TkDodo

Setting pendingMinMs and removing the devtool does not work for me. As long as there is a beforeLoad (which does not return a promise), the application shows nonsense loading component.

No idea how to workaround this.

{
    "@tanstack/react-router": "^1.109.2",
    "@tanstack/router-devtools": "^1.109.2"
}

Since the documentation recommends using beforeLoad as auth guard, it should not have obvious buggy behavior. This library is so superior than current solutions and I don't want to forgo it because of this simple bug.

hanayashiki avatar Feb 23 '25 11:02 hanayashiki

After a few investigation, I find this issue BOTH a tanstack-router & react suspense issue. I find the extra pending time actually a direct result from React. (I'm using react 19.0.0 and tanstack-router 1.109.2)

Given a simple component that suspenses

function IndexComponent() {
  console.log(Date.now());
  if (!thrown) {
    throw new Promise<void>((r) =>
      setTimeout(() => {
        r();
        thrown = true;
      }, 0)
    );
  }
  console.log(Date.now());

  return (
    <div className="p-2">
      <h3>Index</h3>
    </div>
  );
}

export const App: React.FC = () => {
  return (
    <Suspense fallback={"loading"}>
      <IndexComponent />
    </Suspense>
  );
};

Its thrown promise quickly resolves in less than 1ms, but for how long the "loading" appears before the user see "Index"?

According my chrome performance tool, it takes 150ms!

I'm not familiar with React's fiber internals, 150ms, reasonable for remote data fetching, but a painfully long flicker if every "loading" happens on the client-side.

If we stick with the react status-quo, we need to avoid throw new Promise() as long as we can complete the tasks within the current tick.

So, how does tanstack/react-router do? It assumes every beforeLoad returns a promise, and throws it anyways. This causes React to flicker with Suspense with a minimal period of time, even if you set pendingMinMs: 0 or pendingComponent: undefined. (In the later case, it just does not show the pending component and keeps the white screen)

export const MatchInner = React.memo(function MatchInnerImpl({
  matchId,
}: {
  matchId: string
}): any {
  // ....
  // Somehow even if `beforeLoad` is sync, the status is 'pending'
  if (match.status === 'pending') {
    // We're pending, and if we have a minPendingMs, we need to wait for it
    const pendingMinMs =
      route.options.pendingMinMs ?? router.options.defaultPendingMinMs

    if (pendingMinMs && !router.getMatch(match.id)?.minPendingPromise) {
      // Create a promise that will resolve after the minPendingMs
      ...
    }

    throw router.getMatch(match.id)?.loadPromise // <-- Cause React to suspense
  }

  return out
})

Before this is fixed, my suggestion:

  1. For devs: don't use beforeLoad for synchronous functions. Do it in your render function in the old way.
  2. For the authors of this repository: it seems hard to keep beforeLoad the same way because the function does not run synchronously with navigation. My sugguestion is adding a guardSync API that runs synchronously and does not block any rendering at all (or is there already something like that?)

hanayashiki avatar Feb 23 '25 13:02 hanayashiki

This issue still happens for me. Did anyone actually solve this issue?

bojandurmic avatar Mar 27 '25 14:03 bojandurmic

This issue still happens for me. Did anyone actually solve this issue?

don't use loader merely for sync functions

hanayashiki avatar Mar 27 '25 16:03 hanayashiki

@hanayashiki so do you propose just doing redirects based on auth in the render function? Thanks for the reply by the way!

bojandurmic avatar Mar 27 '25 20:03 bojandurmic

I have an async function, but it's too fast because I'm working with indexed db, but this blank screen is too much for me. That's why defaultPendingMinMs: 0 is a solution

letstri avatar Apr 07 '25 10:04 letstri