react-router icon indicating copy to clipboard operation
react-router copied to clipboard

[Feature]: React 18: lazy-loading / useTransition

Open NawarA opened this issue 2 years ago • 13 comments

What is the new or updated feature that you are suggesting?

Lazy loading routes is a common pattern.

Today, if users lazy load routes, they're forced to use a Suspense with a fallback, which causes a flash in their applications.

React 18 comes with useTransition which keeps the former UI static while a new UI is being prepared.

I'd like to request react-router adopt useTransition internally, so developers dont have to manually mark navigation events as transitions. Instead, react-router can automatically manage this internally, benefiting DX and UX.

Why should this feature be included?

Lazy loaded routes will no longer experience a "flash" or "glimmer" effect when they first mount. Meanwhile, apps that use lazy loading can still keep the benefits of code splitting.

NawarA avatar May 12 '22 01:05 NawarA

Currently trying to implement Lazy Loading myself... and running into this flickering issue. I've tried using useTransition myself, but having a hard time wrapping my head around it. Could you show me a code-snippet of what you mean by "manually mark navigation events as transitions"? It would help in the interim :)

locchuong avatar Jun 02 '22 23:06 locchuong

The documentation for upgrading to v6 (https://reactrouter.com/docs/en/v6/upgrading/v5) specifies that react-router is now using useTransition, but it is nowhere to be found in the entire codebase? Is this just completely forgotten? I'm trying to implement lazy loading and data prefetching before showing the next page, but the only thing that seems to be working is the general lazy loading of react for components, falling back to the Suspense fallback component which makes the screen flicker.

I though by reading the documentation that react-router v6 already had full support for suspense and transitions, but it seems it does not?

Nickman87 avatar Jun 16 '22 14:06 Nickman87

I believe that hook is going to be renamed to useNavigation to prevent confusion with React's useTransition.

https://beta.reactrouter.com/en/dev/hooks/use-navigation

kiliman avatar Jun 17 '22 12:06 kiliman

Is there any code-snippets show how to implement lazy load without flickering? I'm a noob and facing the same problem using lazy load with react-router.

cxOrz avatar Jul 05 '22 15:07 cxOrz

Is there any code-snippets show how to implement lazy load without flickering? I'm a noob and facing the same problem using lazy load with react-router.

I'm not sure if you are doing something similar to me, but for me the flicker is caused because the lazy load's fallback is a blank page. Because of this, there is a small time in which this page is loaded.

I need to work on something internally for this, but sorry I wish I had code for avoiding flicker.

It would be nice to make it so instead of falling back to a random react component of our choosing from suspense, we should instead stay on the currently rendered page until React.lazy is finished. Then once that has been lazy loaded, move to that route. In other words if 'null' is specified for the suspense fallback, stay on the currently rendered page until loading is finished, then move.

This feature really is needed.

bearsworth avatar Jul 12 '22 21:07 bearsworth

I have created a demo repo for solving this issue specifically - Route level code splitting in React without screen flickering.

  1. I think it will be easier if there is a way to tell react-router-dom to use transition for a specific Route or Routes. In my demo, I just overrode Link component, but I think startTransition should be called before navigation happens that is inside the useLinkClickHandler function.

  2. And with this concurrent mode, Events like onNavigationStart, onNavigationEnd should be re-added. Because we cannot handle them just by using component life cycles now. That is explained in README.md.

HanMoeHtet avatar Jul 19 '22 17:07 HanMoeHtet

I've used your implementation @HanMoeHtet with some modification in my application and got good results from it.

I've gone a bit deeper and injected the transition code into the navigator as such:

  const { navigator } = React.useContext(UNSAFE_NavigationContext);

  useEffect(() => {
    // Overwrite the default navigator methods to support suspense
    const originalGo = navigator.go;
    const originalPush = navigator.push;
    const originalReplace = navigator.replace;

    const checkStartLoading = (to: number | To) => {
      if (typeof to === "number") {
        if (to !== 0) {
          startLoading();
        }
      } else {
        if (createPath(resolvePath(to)) !== createPath(window.location)) {
          startLoading();
        }
      }
    };

    navigator.go = (delta: number) => {
      checkStartLoading(delta);
      React.startTransition(() => {
        originalGo(delta);
      });
    };
    navigator.push = (to: To, state?: any) => {
      checkStartLoading(to);
      React.startTransition(() => {
        originalPush(to, state);
      });
    };
    navigator.replace = (to: To, state?: any) => {
      checkStartLoading(to);
      React.startTransition(() => {
        originalReplace(to, state);
      });
    };

    return () => {
      // Reset original methods
      navigator.go = originalGo;
      navigator.push = originalPush;
      navigator.replace = originalReplace;
    };
  }, [navigator]);

The startLoading method will trigger a loading animation after 200ms, somewhat the same as the progress component you are using. I'm using toprogress2 instead.

This startTransition call should be included by default in my opinion, or activatable by config for those that want to use it, wih possibly a callback before to trigger animations as we are doing now.

Nickman87 avatar Jul 28 '22 08:07 Nickman87

@Nickman87 @HanMoeHtet Thanks for your demonstration!

Here I am sharing some thoughts about the nested routes.

E.g.:

export const App = () => {
  return (
    <StrictMode>
      <BrowserRouter>
        <Routes>
          <Route path="/" element={<Layout />}>
            <Route index element={
              <Suspense fallback={<div>Loading...</div>}>
                <Home />
              </Suspense>
            } />
            <Route path="about" element={
              <Suspense fallback={<div>Loading...</div>}>
                <About />
              </Suspense>
            } />
            <Route path="*" element={
              <Suspense fallback={<div>Loading...</div>}>
                <NotFound />
              </Suspense>
            } />
          </Route>
        </Routes>
      </BrowserRouter>
    </StrictMode >
  );
};

The component <Home />, <About /> and <NotFound /> are nested inside <Layout />. And <Layout /> is included in the initial page load and shouldn't be included in the Suspense boundary. I only want a Suspense boundary for sub-routes components <Home />, <About /> and <NotFound /> only.

Currently, it is impossible for react-router to include <Suspense /> inside <Routes />. I am currently experimenting with wrapping Suspense outside of <Outlet />. I will update my post after I figure out something.

Update

You should wrap your <Suspense /> outside of <Outlet /> instead of <Route element /> when you are using nested layout/routes. Now all your sub-routes share the same Suspense boundary, and you could apply the approach mentioned by @Nickman87

SukkaW avatar Aug 08 '22 17:08 SukkaW

Here's temporary solution, which works great for me:

const useConcurrentLocation = (): Location & { isPending: boolean } => {
    const originalLocation = useLocation()
    const [location, setLocation] = useState(originalLocation)
    const [isPending, startTransition] = useTransition()

    useEffect(() => {
        if (location.pathname !== originalLocation.pathname || location.search !== originalLocation.search) {
            startTransition(() => {
                setLocation(originalLocation)
            })
        }
    }, [originalLocation.pathname, originalLocation.search])

    return {
        ...location,
        isPending,
    }
}

-------------

const location = useConcurrentLocation()

<App>
    {location.isPending && <LoadingBar />}
    <Routes location={location}>
        {/* Here're your routes  */}
    </Routes>
</App>

Asrover avatar Aug 17 '22 11:08 Asrover

Here's temporary solution, which works great for me:

const useConcurrentLocation = (): Location & { isPending: boolean } => {
    const originalLocation = useLocation()
    const [location, setLocation] = useState(originalLocation)
    const [isPending, startTransition] = useTransition()

    useEffect(() => {
        if (location.pathname !== originalLocation.pathname || location.search !== originalLocation.search) {
            startTransition(() => {
                setLocation(originalLocation)
            })
        }
    }, [originalLocation.pathname, originalLocation.search])

    return {
        ...location,
        isPending,
    }
}

-------------

const location = useConcurrentLocation()

<App>
    {location.isPending && <LoadingBar />}
    <Routes location={location}>
        {/* Here're your routes  */}
    </Routes>
</App>

This caused an infinite loop of remounting in my app :/ But it worked sometimes :)

NawarA avatar Aug 30 '22 19:08 NawarA

This modified version doesnt cause a stackoverflow and does the job for me...

function useConcurrentTransition() {
  const location = useLocation();
  const [oldLocation, setOldLocation] = useState(location);
  const [, startTransition] = useTransition();

  useEffect(() => {
    // if the path or search params change, mark this is a navigational transition
    setOldLocation(oldLocation =>
      oldLocation.pathname !== location.pathname ||
      oldLocation.search !== location.search
        ? location
        : oldLocation
    );
  }, [location]);

  useEffect(() => {
    // tell concurrent mode to pause UI rendering for a moment
    startTransition(() => {});
  }, [oldLocation]);

  return oldLocation;
}
const location = useConcurrentTransition()

<App>
    <Routes location={location}>
        {/* routes  */}
    </Routes>
</App>

Though this should be built into React-Router

NawarA avatar Sep 01 '22 15:09 NawarA

This modified version doesnt cause a stackoverflow and does the job for me...

function useConcurrentTransition() {
  const location = useLocation();
  const [oldLocation, setOldLocation] = useState(location);
  const [, startTransition] = useTransition();

  useEffect(() => {
    // if the path or search params change, mark this is a navigational transition
    setOldLocation(oldLocation =>
      oldLocation.pathname !== location.pathname ||
      oldLocation.search !== location.search
        ? location
        : oldLocation
    );
  }, [location]);

  useEffect(() => {
    // tell concurrent mode to pause UI rendering for a moment
    startTransition(() => {});
  }, [oldLocation]);

  return oldLocation;
}
const location = useConcurrentTransition()

<App>
    <Routes location={location}>
        {/* routes  */}
    </Routes>
</App>

Though this should be built into React-Router

This no longer works with 6.4.0, if you use createBrowserRouter lol

An official solution would be lovely :)

NawarA avatar Sep 16 '22 22:09 NawarA

Would be great if we can use a promise as the route element and get the loading state via useNavigation hook when the promise is resolved. Feel free to improve the idea. element: () => import('./routes/superroute').then((res) => <res.superroute />)

mnlfischer avatar Oct 28 '22 18:10 mnlfischer