router: proper path params during loading

Author: Tobbe

packages/router/src/ActivePageContext.tsx

@@ -1,5 +1,6 @@
 import { useContext } from 'react'
 
+import { LocationContextType } from './location'
 import { createNamedContext } from './util'
 
 export type LoadingState = 'PRE_SHOW' | 'SHOW_LOADING' | 'DONE'
@@ -8,6 +9,7 @@ export type LoadingStateRecord = Record<
   | {
       state: LoadingState
       page: React.ComponentType<unknown>
+      location: LocationContextType
     }
   | undefined
 >

packages/router/src/__tests__/router.test.tsx

@@ -96,7 +96,7 @@ beforeEach(() => {
   Object.keys(routes).forEach((key) => delete routes[key])
 })
 
-describe.only('slow imports', () => {
+describe('slow imports', () => {
   const HomePagePlaceholder = () => <>HomePagePlaceholder</>
   const AboutPagePlaceholder = () => <>AboutPagePlaceholder</>
   const ParamPagePlaceholder = () => <>ParamPagePlaceholder</>
@@ -246,7 +246,7 @@ describe.only('slow imports', () => {
     })
   })
 
-  test.only('useLocation', async () => {
+  test('useLocation', async () => {
     act(() => navigate('/location'))
     const screen = render(<TestRouter />)
     await waitFor(() => screen.getByText('Location Page'))
@@ -271,6 +271,44 @@ describe.only('slow imports', () => {
     // ...followed by the actual page
     await waitFor(() => screen.getByText('About Page'))
   })
+
+  test('path params should never be empty', async () => {
+    const PathParamPage = ({ value }) => {
+      expect(value).not.toBeFalsy()
+      return <p>{value}</p>
+    }
+
+    const TestRouter = () => (
+      <Router pageLoadingDelay={100}>
+        <Route
+          path="/about"
+          page={AboutPage}
+          name="about"
+          whileLoadingPage={AboutPagePlaceholder}
+        />
+        <Route
+          path="/path-param-test/{value}"
+          page={PathParamPage}
+          name="params"
+          whileLoadingPage={ParamPagePlaceholder}
+        />
+      </Router>
+    )
+
+    act(() => navigate('/path-param-test/test_value'))
+    const screen = render(<TestRouter />)
+
+    // First we render the path parameter value "test_value"
+    await waitFor(() => screen.getByText('test_value'))
+
+    act(() => navigate('/about'))
+    // After navigating we should keep displaying the old path value...
+    await waitFor(() => screen.getByText('test_value'))
+    // ...until we switch over to render the about page loading component...
+    await waitFor(() => screen.getByText('AboutPagePlaceholder'))
+    // ...followed by the actual page
+    await waitFor(() => screen.getByText('About Page'))
+  })
 })
 
 describe('inits routes and navigates as expected', () => {

packages/router/src/active-route-loader.tsx

@@ -42,13 +42,12 @@ export const ActiveRouteLoader = ({
   const announcementRef = useRef<HTMLDivElement>(null)
   const waitingFor = useRef<string>('')
   const [loadingState, setLoadingState] = useState<LoadingStateRecord>({
-    [path]: { page: ArlNullPage, state: 'PRE_SHOW' },
+    [path]: { page: ArlNullPage, state: 'PRE_SHOW', location },
   })
   const [renderedChildren, setRenderedChildren] = useState<
     React.ReactNode | undefined
   >(children)
   const [renderedPath, setRenderedPath] = useState(path)
-  const [renderedLocation, setRenderedLocation] = useState({ ...location })
   const isMounted = useIsMounted()
 
   const clearLoadingTimeout = () => {
@@ -82,6 +81,7 @@ export const ActiveRouteLoader = ({
         [path]: {
           page: ArlNullPage,
           state: 'PRE_SHOW',
+          location: { ...location },
         },
       }))
 
@@ -96,11 +96,11 @@ export const ActiveRouteLoader = ({
             [path]: {
               page: whileLoadingPage || ArlWhileLoadingNullPage,
               state: 'SHOW_LOADING',
+              location: { ...location },
             },
           }))
           setRenderedChildren(children)
           setRenderedPath(path)
-          setRenderedLocation({ ...location })
         })
       }, delay)
 
@@ -120,9 +120,9 @@ export const ActiveRouteLoader = ({
             [path]: {
               page: module.default,
               state: 'DONE',
+              location: { ...location },
             },
           }))
-          setRenderedLocation({ ...location })
           setRenderedChildren(children)
           setRenderedPath(path)
           setPageName(name)
@@ -133,6 +133,20 @@ export const ActiveRouteLoader = ({
     if (spec.name !== waitingFor.current) {
       clearLoadingTimeout()
       startPageLoadTransition(spec, delay)
+    } else {
+      // Handle navigating to the same page again, but with different path params
+      setLoadingState((loadingState) => {
+        const page = loadingState[path]?.page || ArlNullPage
+
+        return {
+          ...loadingState,
+          [path]: {
+            page,
+            state: 'DONE',
+            location: { ...location },
+          },
+        }
+      })
     }
 
     return () => {
@@ -153,6 +167,7 @@ export const ActiveRouteLoader = ({
       [path]: {
         state: 'DONE',
         page: PageFromLoader,
+        location: { ...location },
       },
     }
 
@@ -170,10 +185,15 @@ export const ActiveRouteLoader = ({
   }
 
   return (
-    <ParamsProvider path={renderedPath} location={renderedLocation}>
+    <ParamsProvider
+      path={renderedPath}
+      location={loadingState[renderedPath]?.location}
+    >
       <ActivePageContextProvider value={{ loadingState }}>
         <PageLoadingContextProvider
-          value={{ loading: loadingState[path]?.state === 'SHOW_LOADING' }}
+          value={{
+            loading: loadingState[renderedPath]?.state === 'SHOW_LOADING',
+          }}
         >
           {renderedChildren}
           {loadingState[path]?.state === 'DONE' && (

packages/router/src/router.tsx

@@ -73,7 +73,6 @@ const InternalRoute: React.VFC<InternalRouteProps> = ({
   redirect,
   notfound,
 }) => {
-  const location = useLocation()
   const routerState = useRouterState()
   const activePageContext = useActivePageContext()
 
@@ -89,6 +88,12 @@ const InternalRoute: React.VFC<InternalRouteProps> = ({
   // Check for issues with the path.
   validatePath(path)
 
+  const location = activePageContext.loadingState[path]?.location
+
+  if (!location) {
+    throw new Error(`No location for route "${name}"`)
+  }
+
   const { params: pathParams } = matchPath(
     path,
     location.pathname,
@@ -240,7 +245,7 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
   return (
     <RouterContextProvider useAuth={useAuth} paramTypes={paramTypes}>
       {redirect && <Redirect to={replaceParams(redirect, allParams)} />}
-      {!redirect && (
+      {!redirect && page && (
         <ActiveRouteLoader
           path={path}
           spec={normalizePage(page)}
@@ -272,7 +277,7 @@ function analyzeRouterTree(
   paramTypes?: Record<string, ParamType>
 ): {
   root: React.ReactElement | undefined
-  activeRoute: React.ReactElement | undefined
+  activeRoute: React.ReactElement<InternalRouteProps> | undefined
   NotFoundPage: PageType | undefined
 } {
   let NotFoundPage: PageType | undefined = undefined