Router: Remove RouteScanner, don't store all routes in context (#2826)

packages/router/src/Set.tsx

@@ -1,10 +1,9 @@
 import React, { ReactElement, ReactNode, useCallback } from 'react'
 
 import { Redirect } from './links'
-import { useLocation } from './location'
-import { isRoute, routes as namedRoutes } from './router'
+import { InternalRouteProps, isRoute, routes as namedRoutes } from './router'
 import { useRouterState } from './router-context'
-import { flattenAll, matchPath } from './util'
+import { flattenAll } from './util'
 
 type WrapperType<WTProps> = (
   props: WTProps & { children: ReactNode }
@@ -46,7 +45,6 @@ export function Set<WrapperProps>(props: SetProps<WrapperProps>) {
     ...rest
   } = props
   const routerState = useRouterState()
-  const location = useLocation()
   const { loading, isAuthenticated, hasRole } = routerState.useAuth()
 
   if (privateSet && !unauthenticated) {
@@ -62,53 +60,39 @@ export function Set<WrapperProps>(props: SetProps<WrapperProps>) {
   // Make sure `wrappers` is always an array with at least one wrapper component
   const wrappers = Array.isArray(wrap) ? wrap : [wrap ? wrap : IdentityWrapper]
   const flatChildArray = flattenAll(children)
-  const routes = flatChildArray
-    .filter(isRoute)
-    .filter((r) => typeof r.props.path !== 'undefined')
+  const route = flatChildArray.find(
+    (child): child is React.ReactElement<InternalRouteProps> => isRoute(child)
+  )
 
-  for (const route of routes) {
-    const path = route.props.path as string
+  if (privateSet && unauthorized()) {
+    if (loading) {
+      return route?.props?.whileLoading?.() || null
+    } else {
+      const currentLocation =
+        global.location.pathname + encodeURIComponent(global.location.search)
 
-    const { match } = matchPath(path, location.pathname, routerState.paramTypes)
-    if (!match) {
-      continue
-    }
+      // We already have a check for !unauthenticated further up
+      const unauthenticatedPath = namedRoutes[unauthenticated || '']()
 
-    if (privateSet && unauthorized()) {
-      if (loading) {
-        return route.props?.whileLoading?.() || null
-      } else {
-        const currentLocation =
-          global.location.pathname + encodeURIComponent(global.location.search)
-
-        // We already have a check for !unauthenticated further up
-        const unauthenticatedPath = namedRoutes[unauthenticated || '']()
-
-        if (!unauthenticatedPath) {
-          throw new Error(`We could not find a route named ${unauthenticated}`)
-        }
-
-        return (
-          <Redirect
-            to={`${unauthenticatedPath}?redirectTo=${currentLocation}`}
-          />
-        )
+      if (!unauthenticatedPath) {
+        throw new Error(`We could not find a route named ${unauthenticated}`)
       }
-    }
 
-    // Expand and nest the wrapped elements.
-    return (
-      wrappers.reduceRight<ReduceType>((acc, wrapper) => {
-        return React.createElement(wrapper, {
-          ...rest,
-          children: acc ? acc : children,
-        } as SetProps<WrapperProps>)
-      }, undefined) || null
-    )
+      return (
+        <Redirect to={`${unauthenticatedPath}?redirectTo=${currentLocation}`} />
+      )
+    }
   }
 
-  // No match, no render.
-  return null
+  // Expand and nest the wrapped elements.
+  return (
+    wrappers.reduceRight<ReduceType>((acc, wrapper) => {
+      return React.createElement(wrapper, {
+        ...rest,
+        children: acc ? acc : children,
+      } as SetProps<WrapperProps>)
+    }, undefined) || null
+  )
 }
 
 type PrivateProps<P> = Omit<

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

@@ -615,3 +615,52 @@ test('Private is an alias for Set private', async () => {
   await waitFor(() => screen.getByText(/Private Layout \(dark\)/))
   await waitFor(() => screen.getByText(/Private Page/))
 })
+
+test('redirect to last page', async () => {
+  const TestRouter = () => (
+    <Router useAuth={mockUseAuth()}>
+      <Route path="/" page={HomePage} name="home" />
+      <Route path="/about" page={AboutPage} name="about" />
+      <Private unauthenticated="login">
+        <Route path="/private" page={PrivatePage} name="private" />
+      </Private>
+      <Route path="/login" page={LoginPage} name="login" />
+    </Router>
+  )
+  const screen = render(<TestRouter />)
+
+  // starts on home page
+  await waitFor(() => screen.getByText(/Home Page/i))
+
+  // navigate to private page
+  // should redirect to login
+  act(() => navigate(routes.private()))
+
+  await waitFor(() => {
+    expect(screen.queryByText(/Private Page/i)).not.toBeInTheDocument()
+    expect(window.location.pathname).toBe('/login')
+    expect(window.location.search).toBe('?redirectTo=/private')
+    screen.getByText(/Login Page/i)
+  })
+})
+
+test('no location match means nothing is rendered', async () => {
+  const TestRouter = () => (
+    <Router>
+      <Route path="/" page={HomePage} name="home" />
+    </Router>
+  )
+  const screen = render(<TestRouter />)
+
+  // starts on home page
+  await waitFor(() => screen.getByText(/Home Page/i))
+
+  // navigate page that doesn't exist
+  act(() => navigate('/not/found'))
+
+  // wait for rendering
+  // Otherwise adding a NotFound route still makes this test pass
+  await new Promise((r) => setTimeout(r, 200))
+
+  expect(screen.container).toMatchInlineSnapshot('<div />')
+})

packages/router/src/params.tsx

@@ -10,24 +10,22 @@ export interface ParamsContextProps {
 
 export const ParamsContext = createNamedContext<ParamsContextProps>('Params')
 
-export const ParamsProvider: React.FC = ({ children }) => {
-  const { routes, paramTypes } = useRouterState()
+interface Props {
+  path?: string
+}
+
+export const ParamsProvider: React.FC<Props> = ({ path, children }) => {
+  const { paramTypes } = useRouterState()
   const location = useLocation()
 
   let pathParams = {}
   const searchParams = parseSearch(location.search)
 
-  for (const route of routes) {
-    if (route.path) {
-      const { match, params } = matchPath(
-        route.path,
-        location.pathname,
-        paramTypes
-      )
-
-      if (match && typeof params !== 'undefined') {
-        pathParams = params
-      }
+  if (path) {
+    const { match, params } = matchPath(path, location.pathname, paramTypes)
+
+    if (match && typeof params !== 'undefined') {
+      pathParams = params
     }
   }
 

packages/router/src/router-context.tsx

@@ -3,16 +3,13 @@ import React, { useReducer, createContext, useContext } from 'react'
 import { useAuth } from '@redwoodjs/auth'
 
 import type { ParamType } from 'src/internal'
-import { isRoute, PageType } from 'src/router'
-import { flattenAll } from 'src/util'
 
 const DEFAULT_PAGE_LOADING_DELAY = 1000 // milliseconds
 
 export interface RouterState {
   paramTypes?: Record<string, ParamType>
   pageLoadingDelay?: number
   useAuth: typeof useAuth
-  routes: Array<{ name?: string; path?: string; page?: PageType }>
 }
 
 const RouterStateContext = createContext<RouterState | undefined>(undefined)
@@ -25,7 +22,7 @@ const RouterSetContext =
   createContext<React.Dispatch<Partial<RouterState>> | undefined>(undefined)
 
 export interface RouterContextProviderProps
-  extends Omit<RouterState, 'useAuth' | 'routes'> {
+  extends Omit<RouterState, 'useAuth'> {
   useAuth?: typeof useAuth
 }
 
@@ -39,19 +36,10 @@ export const RouterContextProvider: React.FC<RouterContextProviderProps> = ({
   pageLoadingDelay = DEFAULT_PAGE_LOADING_DELAY,
   children,
 }) => {
-  // Create an internal representation of all the routes and paths.
-  const routes = flattenAll(children)
-    .filter(isRoute)
-    .map((route) => {
-      const { name, path, page } = route.props
-      return { name, path, page }
-    })
-
   const [state, setState] = useReducer(stateReducer, {
     useAuth: customUseAuth || useAuth,
     paramTypes,
     pageLoadingDelay,
-    routes,
   })
 
   return (

packages/router/src/router.tsx

@@ -51,7 +51,7 @@ interface NotFoundRouteProps {
   prerender?: boolean
 }
 
-type InternalRouteProps = Partial<
+export type InternalRouteProps = Partial<
   RouteProps & RedirectRouteProps & NotFoundRouteProps
 >
 
@@ -132,7 +132,25 @@ const Router: React.FC<RouterProps> = ({
   paramTypes,
   pageLoadingDelay,
   children,
+}) => (
+  <LocationProvider>
+    <LocationAwareRouter
+      useAuth={useAuth}
+      paramTypes={paramTypes}
+      pageLoadingDelay={pageLoadingDelay}
+    >
+      {children}
+    </LocationAwareRouter>
+  </LocationProvider>
+)
+
+const LocationAwareRouter: React.FC<RouterProps> = ({
+  useAuth,
+  paramTypes,
+  pageLoadingDelay,
+  children,
 }) => {
+  const { pathname } = useLocation()
   const flatChildArray = flattenAll(children)
   const shouldShowSplash =
     flatChildArray.length === 1 &&
@@ -158,22 +176,55 @@ const Router: React.FC<RouterProps> = ({
     }
   })
 
+  let activeRoute = undefined
+  let NotFoundPage: PageType | undefined = undefined
+
+  const activeChildren = activeRouteTree(children, (child) => {
+    if (child.props.path) {
+      const { match } = matchPath(child.props.path, pathname, paramTypes)
+
+      if (match) {
+        activeRoute = child
+
+        // No need to loop further. As soon as we have a matching route we have
+        // all the info we need
+        return true
+      }
+    }
+
+    if (child.props.notfound && child.props.page) {
+      NotFoundPage = child.props.page
+    }
+
+    return false
+  })
+
   return (
     <RouterContextProvider
       useAuth={useAuth}
       paramTypes={paramTypes}
       pageLoadingDelay={pageLoadingDelay}
     >
-      <LocationProvider>
-        <ParamsProvider>
-          <RouteScanner>{children}</RouteScanner>
-        </ParamsProvider>
-      </LocationProvider>
+      {/* TS doesn't "see" the assignment to `activeRoute` inside the callback
+          above. So it's type is `never`. And you can't access attributes
+          (props in this case) on `never`. There is an open issue about not
+          seeing the assignment */}
+      {/* @ts-expect-error - https://github.com/microsoft/TypeScript/issues/11498 */}
+      <ParamsProvider path={activeRoute?.props?.path}>
+        {!activeRoute && NotFoundPage ? (
+          <PageLoader
+            spec={normalizePage(NotFoundPage)}
+            delay={pageLoadingDelay}
+          />
+        ) : (
+          activeRoute && activeChildren
+        )}
+      </ParamsProvider>
     </RouterContextProvider>
   )
 }
 
-/**
+/*
  * Find the active (i.e. first matching) route and discard any other routes.
  * Also, keep any <Set>s wrapping the active route.
  */
@@ -222,51 +273,6 @@ function activeRouteTree(
   )
 }
 
-const RouteScanner: React.FC = ({ children }) => {
-  const location = useLocation()
-  const routerState = useRouterState()
-
-  let foundMatchingRoute = false
-  let NotFoundPage: PageType | undefined = undefined
-
-  const filteredChildren = activeRouteTree(children, (child) => {
-    const { path } = child.props
-
-    if (path) {
-      const { match } = matchPath(
-        path,
-        location.pathname,
-        routerState.paramTypes
-      )
-
-      if (match) {
-        foundMatchingRoute = true
-
-        // No need to loop further. As soon as we have a matching route we have
-        // all the info we need
-        return true
-      }
-    }
-
-    if (child.props.notfound && child.props.page) {
-      NotFoundPage = child.props.page
-    }
-
-    return false
-  })
-
-  if (!foundMatchingRoute && NotFoundPage) {
-    return (
-      <PageLoader
-        spec={normalizePage(NotFoundPage)}
-        delay={routerState.pageLoadingDelay}
-      />
-    )
-  }
-
-  return <>{filteredChildren}</>
-}
-
 function isSpec(specOrPage: Spec | React.ComponentType): specOrPage is Spec {
   return (specOrPage as Spec).loader !== undefined
 }