fix: old data kept on new route during transition (#730)

Co-authored-by: Marco Pasqualetti <marco.pasqualetti@live.com>

Author: Valerioageno

packages/tuono-router/src/components/RouteMatch.spec.tsx

@@ -4,6 +4,7 @@ import { cleanup, render, screen } from '@testing-library/react'
 import { Route } from '../route'
 import type { RouteComponent, RouteProps } from '../types'
 import { useServerPayloadData } from '../hooks/useServerPayloadData'
+import { useRouterContext } from '../components/RouterContext'
 
 import { RouteMatch } from './RouteMatch'
 
@@ -47,16 +48,21 @@ vi.mock('../hooks/useServerPayloadData', () => ({
   useServerPayloadData: vi.fn(),
 }))
 
+vi.mock('../components/RouterContext', () => ({
+  useRouterContext: vi.fn(),
+}))
+
 const useServerPayloadDataMock = vi.mocked(useServerPayloadData)
+const useRouterContextMock = vi.mocked(useRouterContext)
 
 describe('<RouteMatch />', () => {
   afterEach(cleanup)
 
   it('should correctly render nested routes', () => {
-    useServerPayloadDataMock.mockReturnValue({
-      data: { some: 'data' },
-      isLoading: false,
-    })
+    useServerPayloadDataMock.mockReturnValue({ data: { some: 'data' } })
+
+    // @ts-expect-error only isTransitioning is used by RouteMatch
+    useRouterContextMock.mockReturnValue({ isTransitioning: false })
 
     render(<RouteMatch route={route} serverInitialData={{}} />)
 
@@ -81,13 +87,15 @@ describe('<RouteMatch />', () => {
     )
   })
 
-  it('should return null data when while loading', () => {
-    useServerPayloadDataMock.mockReturnValue({
-      data: { some: 'data' },
-      isLoading: true,
-    })
+  it('should correctly handle loading transition', () => {
+    useServerPayloadDataMock.mockReturnValue({ data: { some: 'data' } })
 
-    render(<RouteMatch route={route} serverInitialData={{}} />)
+    // @ts-expect-error only isTransitioning is used by RouteMatch
+    useRouterContextMock.mockReturnValue({ isTransitioning: true })
+
+    const { rerender } = render(
+      <RouteMatch route={route} serverInitialData={{}} />,
+    )
 
     expect(screen.getByTestId('root')).toMatchInlineSnapshot(
       `
@@ -106,5 +114,30 @@ describe('<RouteMatch />', () => {
       </div>
     `,
     )
+
+    // @ts-expect-error only isTransitioning is used by RouteMatch
+    useRouterContextMock.mockReturnValue({ isTransitioning: false })
+
+    rerender(<RouteMatch route={route} serverInitialData={{}} />)
+
+    expect(screen.getByTestId('root')).toMatchInlineSnapshot(
+      `
+      <div
+        data-testid="root"
+      >
+        root route
+        <div
+          data-testid="parent"
+        >
+          parent route
+          <div
+            data-testid="current"
+          >
+            {"some":"data"}
+          </div>
+        </div>
+      </div>
+    `,
+    )
   })
 })

packages/tuono-router/src/components/RouteMatch.tsx

@@ -2,8 +2,11 @@ import type { JSX } from 'react'
 import { memo, Suspense, useMemo } from 'react'
 
 import type { Route } from '../route'
+
 import { useServerPayloadData } from '../hooks/useServerPayloadData'
 
+import { useRouterContext } from './RouterContext'
+
 interface RouteMatchProps<TServerPayloadData = unknown> {
   route: Route
   // User defined server side props
@@ -19,21 +22,22 @@ export const RouteMatch = ({
   route,
   serverInitialData,
 }: RouteMatchProps): JSX.Element => {
-  const { data, isLoading } = useServerPayloadData(route, serverInitialData)
+  const { data } = useServerPayloadData(route, serverInitialData)
+  const { isTransitioning } = useRouterContext()
 
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const routes = useMemo(() => loadParentComponents(route), [route.id])
 
-  const routeData = isLoading ? null : data
+  const routeData = isTransitioning ? null : data
 
   return (
     <TraverseRootComponents
       routes={routes}
       data={routeData}
-      isLoading={isLoading}
+      isLoading={isTransitioning}
     >
       <Suspense>
-        <route.component data={routeData} isLoading={isLoading} />
+        <route.component data={routeData} isLoading={isTransitioning} />
       </Suspense>
     </TraverseRootComponents>
   )

packages/tuono-router/src/components/RouterContext.tsx

@@ -1,4 +1,11 @@
-import { createContext, useState, useEffect, useContext, useMemo } from 'react'
+import {
+  createContext,
+  useState,
+  useEffect,
+  useContext,
+  useMemo,
+  useCallback,
+} from 'react'
 import type { ReactNode } from 'react'
 
 import type { Router } from '../router'
@@ -17,7 +24,9 @@ export interface ParsedLocation {
 interface RouterContextValue {
   router: Router
   location: ParsedLocation
+  isTransitioning: boolean
   updateLocation: (loc: ParsedLocation) => void
+  stopTransitioning: () => void
 }
 
 const RouterContext = createContext({} as RouterContextValue)
@@ -64,6 +73,9 @@ export function RouterContextProvider({
   const [location, setLocation] = useState<ParsedLocation>(() =>
     getInitialLocation(serverInitialLocation),
   )
+  // Global state to track whether a page transition is in progress.
+  // Set to `false` once the page is fully loaded, including server-side data.
+  const [isTransitioning, setIsTransitioning] = useState<boolean>(false)
 
   /**
    * Listen browser navigation events
@@ -91,13 +103,24 @@ export function RouterContextProvider({
     }
   }, [])
 
+  const updateLocation = useCallback((newLocation: ParsedLocation): void => {
+    setIsTransitioning(true)
+    setLocation(newLocation)
+  }, [])
+
+  const stopTransitioning = useCallback((): void => {
+    setIsTransitioning(false)
+  }, [])
+
   const contextValue: RouterContextValue = useMemo(
     () => ({
       router,
       location,
-      updateLocation: setLocation,
+      isTransitioning,
+      updateLocation,
+      stopTransitioning,
     }),
-    [location, router],
+    [location, router, isTransitioning, updateLocation, stopTransitioning],
   )
 
   return (

packages/tuono-router/src/hooks/useServerPayloadData.ts

@@ -5,8 +5,6 @@ import { fromUrlToParsedLocation } from '../utils/from-url-to-parsed-location'
 
 import { useRouterContext } from '../components/RouterContext'
 
-const isServer = typeof document === 'undefined'
-
 interface TuonoApi {
   data?: unknown
   info: {
@@ -22,7 +20,6 @@ const fetchClientSideData = async (): Promise<TuonoApi> => {
 
 interface UseServerPayloadDataResult<TData> {
   data: TData
-  isLoading: boolean
 }
 
 /*
@@ -37,15 +34,7 @@ export function useServerPayloadData<TServerPayloadData>(
   serverInitialData: TServerPayloadData,
 ): UseServerPayloadDataResult<TServerPayloadData> {
   const isFirstRendering = useRef<boolean>(true)
-  const { location, updateLocation } = useRouterContext()
-  const [isLoading, setIsLoading] = useState<boolean>(
-    // Force loading if has handler
-    !!route.options.hasHandler &&
-      // Avoid loading on the server
-      !isServer &&
-      // Avoid loading if first rendering
-      !isFirstRendering.current,
-  )
+  const { location, updateLocation, stopTransitioning } = useRouterContext()
 
   const [data, setData] = useState<TServerPayloadData | undefined>(
     serverInitialData,
@@ -56,14 +45,14 @@ export function useServerPayloadData<TServerPayloadData>(
     // props are already bundled by the SSR
     if (isFirstRendering.current) {
       isFirstRendering.current = false
+      stopTransitioning()
       return
     }
     // After client side routing load again the remote data
     if (route.options.hasHandler) {
       // The error management is already handled inside the IIFE
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       ;(async (): Promise<void> => {
-        setIsLoading(true)
         try {
           const response = await fetchClientSideData()
           if (response.info.redirect_destination) {
@@ -84,16 +73,23 @@ export function useServerPayloadData<TServerPayloadData>(
         } catch (error) {
           throw Error('Failed loading Server Side Data', { cause: error })
         } finally {
-          setIsLoading(false)
+          stopTransitioning()
         }
       })()
+    } else {
+      stopTransitioning()
     }
 
     // Clean up the data when changing route
     return (): void => {
       setData(undefined)
     }
-  }, [location.pathname, route.options.hasHandler, updateLocation])
+  }, [
+    location.pathname,
+    route.options.hasHandler,
+    updateLocation,
+    stopTransitioning,
+  ])
 
-  return { isLoading, data: data as TServerPayloadData }
+  return { data: data as TServerPayloadData }
 }