Fix wrap and set pass thru props

packages/router/Todo.todo

@@ -14,7 +14,17 @@ Restore existing functionality:
 
 Props on Sets/Private/etc.:
       ✔ whileLoading @done(23-03-08 12:10)
-      ☐ and whileLoadingAuth
+      ☐ Auth related
+      Do we even need this? When server rendering, we won't load auth
+        ☐ error on unauthorized
+        ☐ whileLoadingAuth component
+        ☐ redirect on unauthenticated, and error if route not found
+        ☐ error on missing parameters
+      ✔ wrap components not rendered @done(23-03-21 15:21)
+
+
+While loading:
+  ☐ Can we wrap the whileLoadingPage in a component that updates itself with startTransition?
 
 Redirects:
   ☐ Handling the redirect prop in a Route client side

packages/router/src/Set.tsx

@@ -5,7 +5,7 @@ import { usePageLoadingContext } from './PageLoadingContext'
 import { routes as namedRoutes } from './router'
 import { useRouterState } from './router-context'
 
-type WrapperType<WTProps> = (
+export type WrapperType<WTProps> = (
   props: WTProps & { children: ReactNode }
 ) => ReactElement | null
 

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

@@ -1,6 +1,7 @@
 import React, { isValidElement } from 'react'
 
 import { Route, Router } from '../router'
+import { Set } from '../Set'
 import { analyzeRoutes } from '../util'
 
 const FakePage = () => <h1>Fake Page</h1>
@@ -21,12 +22,34 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
       currentPathName: '/',
     })
   test('Should return namePathMap and hasHomeRoute correctly', () => {
-    expect(namePathMap).toEqual(
+    expect(Object.keys(namePathMap)).toEqual([
+      'hello',
+      'world',
+      'recipeById',
+      'home',
+    ])
+
+    expect(namePathMap.hello).toEqual(
+      expect.objectContaining({
+        name: 'hello',
+        page: FakePage,
+        path: '/hello',
+      })
+    )
+
+    expect(namePathMap.world).toEqual(
       expect.objectContaining({
-        '/world': { name: 'world', path: '/world' },
-        '/hello': { name: 'hello', path: '/hello' },
-        '/recipe/{id}': { name: 'recipeById', path: '/recipe/{id}' },
-        '/': { name: 'home', path: '/' },
+        name: 'world',
+        page: FakePage,
+        path: '/world',
+      })
+    )
+
+    expect(namePathMap.recipeById).toEqual(
+      expect.objectContaining({
+        name: 'recipeById',
+        page: FakePage,
+        path: '/recipe/{id}',
       })
     )
 
@@ -55,24 +78,101 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
     expect(activeRouteName).toBeDefined()
     expect(activeRouteName).toBe('recipeById')
   })
-})
 
-test('No home Route', () => {
-  const CheckRoutes = (
-    <Router>
-      <Route path="/iGots" name="iGots" page={FakePage} />
-      <Route path="/noHome" name="noHome" page={FakePage} />
-    </Router>
-  )
+  test('No home Route', () => {
+    const CheckRoutes = (
+      <Router>
+        <Route path="/iGots" name="iGots" page={FakePage} />
+        <Route path="/noHome" name="noHome" page={FakePage} />
+      </Router>
+    )
 
-  const { namePathMap, namedRoutesMap, hasHomeRoute } = analyzeRoutes(
-    CheckRoutes.props.children,
-    {
+    const { namePathMap, namedRoutesMap, hasHomeRoute } = analyzeRoutes(
+      CheckRoutes.props.children,
+      {
+        currentPathName: '/',
+      }
+    )
+
+    expect(Object.keys(namedRoutesMap).length).toEqual(2)
+    expect(Object.keys(namePathMap).length).toEqual(2)
+    expect(hasHomeRoute).toBe(false)
+  })
+
+  test('Creates setWrapper map', () => {
+    const WrapperX = ({ children }) => (
+      <>
+        <h1>WrapperA</h1>
+        {children}
+      </>
+    )
+
+    const WrapperY = ({ children }) => (
+      <>
+        <h1>WrapperY</h1>
+        {children}
+      </>
+    )
+
+    const Simple = (
+      <Router>
+        <Set wrap={[WrapperX]} id="set-one" passThruProp="bazinga">
+          <Route path="/a" name="routeA" page={FakePage} />
+          <Set wrap={[WrapperY]} id="set-two" theme="blue">
+            <Route name="routeB" path="/b" page={FakePage} />
+            <Route name="routeC" path="/c" page={FakePage} />
+          </Set>
+        </Set>
+      </Router>
+    )
+
+    const { namePathMap } = analyzeRoutes(Simple.props.children, {
       currentPathName: '/',
-    }
-  )
+    })
 
-  expect(Object.keys(namedRoutesMap).length).toEqual(2)
-  expect(Object.keys(namePathMap).length).toEqual(2)
-  expect(hasHomeRoute).toBe(false)
+    expect(namePathMap.routeA).toEqual(
+      expect.objectContaining({
+        redirect: null,
+        name: 'routeA',
+        path: '/a',
+        whileLoadingPage: undefined,
+        wrappers: [WrapperX],
+        // Props passed through from set
+        setProps: {
+          id: 'set-one',
+          passThruProp: 'bazinga',
+        },
+      })
+    )
+
+    expect(namePathMap.routeB).toEqual(
+      expect.objectContaining({
+        redirect: null,
+        name: 'routeB',
+        path: '/b',
+        whileLoadingPage: undefined,
+        wrappers: [WrapperX, WrapperY], // both wrappers
+        setProps: {
+          id: 'set-two',
+          theme: 'blue',
+          passThruProp: 'bazinga', // from the first set
+        },
+      })
+    )
+
+    expect(namePathMap.routeC).toEqual(
+      expect.objectContaining({
+        redirect: null,
+        name: 'routeC',
+        path: '/c',
+        whileLoadingPage: undefined,
+        wrappers: [WrapperX, WrapperY], // both wrappers
+        setProps: {
+          id: 'set-two',
+          theme: 'blue',
+          passThruProp: 'bazinga', // from the first set
+        },
+      })
+    )
+  })
 })

packages/router/src/router.tsx

@@ -1,4 +1,4 @@
-import React, { ReactNode, ReactElement, useMemo } from 'react'
+import React, { ReactNode, ReactElement, useMemo, memo } from 'react'
 
 import { ActiveRouteLoader } from './active-route-loader'
 import { Redirect } from './links'
@@ -98,6 +98,8 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
     () =>
       analyzeRoutes(children, {
         currentPathName: location.pathname,
+        // @TODO We haven't handled this with SSR/Streaming yet.
+        // May need a babel plugin to extract userParamTypes from Routes.tsx
         userParamTypes: paramTypes,
       }),
     [location.pathname, children, paramTypes]
@@ -143,8 +145,15 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
     return null
   }
 
-  const { path, page, name, redirect, whileLoadingPage } =
-    namePathMap[activeRouteName]
+  const {
+    path,
+    page,
+    name,
+    redirect,
+    whileLoadingPage,
+    wrappers = [],
+    setProps,
+  } = namePathMap[activeRouteName]
 
   if (!path) {
     throw new Error(`Route "${name}" needs to specify a path`)
@@ -166,19 +175,51 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
       <ParamsProvider allParams={allParams}>
         {redirect && <Redirect to={replaceParams(redirect, allParams)} />}
         {!redirect && page && (
-          <ActiveRouteLoader
-            path={path}
-            spec={normalizePage(page)}
-            delay={pageLoadingDelay}
-            params={allParams}
-            whileLoadingPage={whileLoadingPage as any}
+          <WrappedPage
+            wrappers={wrappers}
+            routeLoaderElement={
+              <ActiveRouteLoader
+                path={path}
+                spec={normalizePage(page as any)}
+                delay={pageLoadingDelay}
+                params={allParams}
+                whileLoadingPage={whileLoadingPage as any}
+                {...setProps}
+              />
+            }
+            setProps={setProps}
           />
         )}
       </ParamsProvider>
     </RouterContextProvider>
   )
 }
 
+interface WrappedPageProps {
+  wrappers: ReactNode[]
+  routeLoaderElement: ReactNode
+  setProps: Record<any, any>
+}
+
+const WrappedPage = memo(
+  ({ wrappers, routeLoaderElement, setProps }: WrappedPageProps) => {
+    if (wrappers.length > 0) {
+      // If wrappers exist e.g. [a,b,c] -> <a><b><c><routeLoader></c></b></a>
+      return wrappers.reduceRight((acc, wrapper) => {
+        return React.createElement(
+          wrapper as any,
+          {
+            ...setProps,
+          },
+          acc ? acc : routeLoaderElement
+        )
+      }, undefined) as any
+    }
+
+    return routeLoaderElement
+  }
+)
+
 export {
   Router,
   Route,

packages/router/src/util.ts

@@ -449,6 +449,8 @@ interface AnalyzedRoute {
   whileLoadingPage?: WhileLoadingPage
   page: PageType | null
   redirect: string | null
+  wrappers: ReactNode[]
+  setProps: Record<any, any>
 }
 
 export function analyzeRoutes(
@@ -462,10 +464,19 @@ export function analyzeRoutes(
   let NotFoundPage: PageType | undefined
   let activeRouteName: string | undefined
 
-  const recurseThroughRouter = (
-    nodes: ReturnType<typeof Children.toArray>,
+  interface RecurseParams {
+    nodes: ReturnType<typeof Children.toArray>
     whileLoadingPageFromSet?: WhileLoadingPage
-  ) => {
+    wrappersFromSet?: ReactNode[]
+    propsFromSet?: Record<any, any> // we don't know, or care about what props users are passing donw
+  }
+
+  const recurseThroughRouter = ({
+    nodes,
+    whileLoadingPageFromSet,
+    wrappersFromSet = [],
+    propsFromSet = {},
+  }: RecurseParams) => {
     nodes.forEach((node) => {
       if (isValidRoute(node)) {
         // Just for readability
@@ -475,6 +486,8 @@ export function analyzeRoutes(
         if (isNotFoundRoute(route)) {
           NotFoundPage = route.props.page
           // Dont add notFound routes to the maps, and exit early
+          // @TODO: We may need to add it to the map, because you can in
+          // theory wrap a notfound page in a Set wrapper
           return
         }
 
@@ -493,6 +506,8 @@ export function analyzeRoutes(
             name,
             path,
             page: null, // Redirects don't need pages. We set this to null for consistency
+            wrappers: wrappersFromSet,
+            setProps: propsFromSet,
           }
         }
 
@@ -518,6 +533,8 @@ export function analyzeRoutes(
             whileLoadingPage:
               route.props.whileLoadingPage || whileLoadingPageFromSet,
             page: page,
+            wrappers: wrappersFromSet,
+            setProps: propsFromSet,
           }
 
           // e.g. namedRoutesMap.homePage = () => '/home'
@@ -527,20 +544,36 @@ export function analyzeRoutes(
 
       // @NOTE: A <Private> is also a Set
       if (isSetNode(node)) {
-        if (node.props.children) {
-          recurseThroughRouter(
-            Children.toArray(node.props.children),
+        const {
+          children,
+          whileLoadingPage: whileLoadingPageFromCurrentSet,
+          wrap: wrapFromCurrentSet,
+          ...otherPropsFromCurrentSet
+        } = node.props
+
+        const wrapperComponentsArray = Array.isArray(wrapFromCurrentSet)
+          ? wrapFromCurrentSet
+          : [wrapFromCurrentSet]
+
+        if (children) {
+          recurseThroughRouter({
+            nodes: Children.toArray(children),
             // When there's a whileLoadingPage prop on a Set, we pass it down to all its children
             // If the parent node was also a Set with whileLoadingPage, we pass it down. The child's whileLoadingPage
             // will always take precedence over the parent's
-            node.props.whileLoadingPage || whileLoadingPageFromSet
-          )
+            whileLoadingPageFromSet:
+              whileLoadingPageFromCurrentSet || whileLoadingPageFromSet,
+            wrappersFromSet: [...wrappersFromSet, ...wrapperComponentsArray],
+            propsFromSet: { ...propsFromSet, ...otherPropsFromCurrentSet }, // Current one takes precedence
+            // @TODO pass this in
+            // otherPropsFromSet: otherSetProps,
+          })
         }
       }
     })
   }
 
-  recurseThroughRouter(Children.toArray(children))
+  recurseThroughRouter({ nodes: Children.toArray(children) })
 
   return {
     namePathMap,