diff --git a/packages/router/src/__tests__/analyzeRoutes.test.tsx b/packages/router/src/__tests__/analyzeRoutes.test.tsx
index 8da79e36ce1d..95a8b3426d54 100644
--- a/packages/router/src/__tests__/analyzeRoutes.test.tsx
+++ b/packages/router/src/__tests__/analyzeRoutes.test.tsx
@@ -144,10 +144,12 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
whileLoadingPage: undefined,
wrappers: [WrapperX],
// Props passed through from set
- setProps: {
- id: 'set-one',
- passThruProp: 'bazinga',
- },
+ setProps: [
+ {
+ id: 'set-one',
+ passThruProp: 'bazinga',
+ },
+ ],
})
)
@@ -158,11 +160,16 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
path: '/b',
whileLoadingPage: undefined,
wrappers: [WrapperX, WrapperY], // both wrappers
- setProps: {
- id: 'set-two', // the id gets overwritten by the second Set
- theme: 'blue',
- passThruProp: 'bazinga', // from the first set
- },
+ setProps: [
+ {
+ id: 'set-one',
+ passThruProp: 'bazinga',
+ },
+ {
+ id: 'set-two',
+ theme: 'blue',
+ },
+ ],
})
)
@@ -173,11 +180,16 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
path: '/c',
whileLoadingPage: undefined,
wrappers: [WrapperX, WrapperY], // both wrappers
- setProps: {
- id: 'set-two',
- theme: 'blue',
- passThruProp: 'bazinga', // from the first set
- },
+ setProps: [
+ {
+ id: 'set-one',
+ passThruProp: 'bazinga',
+ },
+ {
+ id: 'set-two',
+ theme: 'blue',
+ },
+ ],
})
)
})
@@ -276,10 +288,12 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
page: FakePage,
wrappers: [],
setId: 1,
- setProps: {
- private: true,
- unauthenticated: 'home',
- },
+ setProps: [
+ {
+ private: true,
+ unauthenticated: 'home',
+ },
+ ],
})
})
@@ -310,4 +324,63 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
expect(namedRoutesMap.simple()).toBe('/simple')
expect(namedRoutesMap.rdSimple()).toBe('/rdSimple')
})
+
+ test('Redirect routes analysis', () => {
+ const HomePage = () =>
Home Page
+ const PrivateAdminPage = () => Private Admin Page
+ const PrivateEmployeePage = () => Private Employee Page
+ const PrivateNoRolesAssigned = () => Private Employee Page
+
+ const RedirectedRoutes = (
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ )
+
+ const { pathRouteMap, namedRoutesMap } = analyzeRoutes(
+ RedirectedRoutes.props.children,
+ {
+ currentPathName: '/simple',
+ }
+ )
+
+ console.log(JSON.stringify({ pathRouteMap, namedRoutesMap }, null, 3))
+
+ // expect(pathRouteMap['/simple'].redirect).toBe('/rdSimple')
+ // expect(pathRouteMap['/rdSimple'].redirect).toBeFalsy()
+
+ // // @TODO true for now, but we may not allow names on a redirect route
+ // expect(Object.keys(namedRoutesMap).length).toBe(2)
+ // expect(namedRoutesMap.simple()).toBe('/simple')
+ // expect(namedRoutesMap.rdSimple()).toBe('/rdSimple')
+ })
})
diff --git a/packages/router/src/__tests__/router.test.tsx b/packages/router/src/__tests__/router.test.tsx
index ec00943cbac9..a44c738f99cb 100644
--- a/packages/router/src/__tests__/router.test.tsx
+++ b/packages/router/src/__tests__/router.test.tsx
@@ -20,32 +20,32 @@ jest.mock('../util', () => {
import React, { useEffect, useState } from 'react'
+import '@testing-library/jest-dom/extend-expect'
import {
- render,
- waitFor,
act,
- fireEvent,
configure,
+ fireEvent,
+ render,
+ waitFor,
} from '@testing-library/react'
-import '@testing-library/jest-dom/extend-expect'
import type { AuthContextInterface } from '@redwoodjs/auth'
import {
- Router,
- Route,
- Private,
- Redirect,
+ back,
routes as generatedRoutes,
Link,
navigate,
- back,
+ Private,
+ Redirect,
+ Route,
+ Router,
usePageLoadingContext,
} from '../'
import { useLocation } from '../location'
import { useParams } from '../params'
import { Set } from '../Set'
-import type { Spec, GeneratedRoutesMap } from '../util'
+import type { GeneratedRoutesMap, Spec } from '../util'
/** running into intermittent test timeout behavior in https://github.com/redwoodjs/redwood/pull/4992
attempting to work around by bumping the default timeout of 5000 */
@@ -98,7 +98,7 @@ function createDummyAuthContextValues(
interface MockAuth {
isAuthenticated?: boolean
loading?: boolean
- hasRole?: boolean
+ hasRole?: boolean | ((role: string[]) => boolean)
loadingTimeMs?: number
}
@@ -138,7 +138,7 @@ const mockUseAuth =
return createDummyAuthContextValues({
loading: authLoading,
isAuthenticated: authIsAuthenticated,
- hasRole: () => hasRole,
+ hasRole: typeof hasRole === 'boolean' ? () => hasRole : hasRole,
})
}
@@ -1593,3 +1593,199 @@ describe('Unauthorized redirect error messages', () => {
)
})
})
+
+describe('Multiple nested private sets', () => {
+ const HomePage = () => Home Page
+ const PrivateNoRolesAssigned = () => Private No Roles Page
+ const PrivateEmployeePage = () => Private Employee Page
+ const PrivateAdminPage = () => Private Admin Page
+
+ const LevelLayout = ({ children, level }) => (
+
+ Level: {level}
+ {children}
+
+ )
+
+ const TestRouter = ({ useAuthMock }) => (
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ )
+
+ test('is authenticated but does not have matching roles', async () => {
+ const screen = render(
+
+ )
+
+ act(() => navigate('/employee'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Private No Roles Page`)).toBeInTheDocument()
+ expect(screen.queryByText(`Level: 1`)).toBeInTheDocument()
+ })
+ })
+
+ test('is not authenticated', async () => {
+ const screen = render(
+
+ )
+
+ act(() => navigate('/employee'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Home Page`)).toBeInTheDocument()
+ expect(screen.queryByText(`Level`)).not.toBeInTheDocument()
+ })
+ })
+
+ test('is authenticated and has a matching role', async () => {
+ const screen = render(
+ {
+ return role.includes('ADMIN')
+ },
+ })}
+ />
+ )
+
+ act(() => navigate('/admin'))
+ await waitFor(() => {
+ expect(screen.queryByText(`Private Admin Page`)).toBeInTheDocument()
+ expect(screen.queryByText(`Level: 3`)).toBeInTheDocument()
+ })
+ })
+
+ test('returns the correct page if has a matching role', async () => {
+ const screen = render(
+ {
+ return role.includes('ADMIN')
+ },
+ })}
+ />
+ )
+
+ act(() => navigate('/admin'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Private Admin Page`)).toBeInTheDocument()
+ expect(screen.queryByText(`Level: 3`)).toBeInTheDocument()
+ })
+ })
+})
+
+describe('Multiple nested private sets', () => {
+ const HomePage = () => Home Page
+ const Page = () => Page
+
+ const DebugLayout = (props) => {
+ return (
+
+
Theme: {props.theme}
+
Other Prop: {props.otherProp}
+
Page Level: {props.level}
+ {props.children}
+
+ )
+ }
+
+ const TestRouter = () => (
+
+
+
+
+
+
+
+
+
+
+
+
+ )
+
+ test('level 1, matches expected props', async () => {
+ const screen = render()
+
+ act(() => navigate('/level1'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Theme: blue`)).toBeInTheDocument()
+ expect(screen.queryByText(`Page Level: 1`)).toBeInTheDocument()
+ })
+ })
+
+ test('level 2, should override level 1', async () => {
+ const screen = render()
+
+ act(() => navigate('/level2'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Theme: red`)).toBeInTheDocument()
+ expect(screen.queryByText(`Other Prop: bazinga`)).toBeInTheDocument()
+ expect(screen.queryByText(`Page Level: 2`)).toBeInTheDocument()
+ })
+ })
+
+ test('level 3, should override level 1 & 2 and pass through other props', async () => {
+ const screen = render()
+
+ act(() => navigate('/level3'))
+
+ await waitFor(() => {
+ expect(screen.queryByText(`Theme: green`)).toBeInTheDocument()
+ expect(screen.queryByText(`Other Prop: bazinga`)).toBeInTheDocument()
+ expect(screen.queryByText(`Page Level: 3`)).toBeInTheDocument()
+ })
+ })
+})
diff --git a/packages/router/src/router.tsx b/packages/router/src/router.tsx
index 8f8d5785987b..4e6c71c7183b 100644
--- a/packages/router/src/router.tsx
+++ b/packages/router/src/router.tsx
@@ -188,7 +188,6 @@ const LocationAwareRouter: React.FC = ({
spec={normalizePage(page as any)}
params={allParams}
whileLoadingPage={whileLoadingPage as any}
- {...setProps}
/>
}
setProps={setProps}
@@ -203,7 +202,7 @@ const LocationAwareRouter: React.FC = ({
interface WrappedPageProps {
wrappers: Wrappers
routeLoaderElement: ReactNode
- setProps: Record
+ setProps: Record[]
}
/**
@@ -220,27 +219,50 @@ const WrappedPage = memo(
({ wrappers, routeLoaderElement, setProps }: WrappedPageProps) => {
// @NOTE: don't mutate the wrappers array, it causes full page re-renders
// Instead just create a new array with the AuthenticatedRoute wrapper
+
+ // we need to pass the setProps from each set to each wrapper
let wrappersWithAuthMaybe = wrappers
- if (setProps.private) {
- if (!setProps.unauthenticated) {
- throw new Error(
- 'You must specify an `unauthenticated` route when marking a Route as private'
- )
- }
+ const reveresedSetProps = [...setProps].reverse()
- wrappersWithAuthMaybe = [AuthenticatedRoute, ...wrappers]
- }
+ reveresedSetProps
+ // @MARK note the reverse() here, because we spread wrappersWithAuthMaybe
+ .forEach((propsFromSet) => {
+ if (propsFromSet.private) {
+ if (!propsFromSet.unauthenticated) {
+ throw new Error(
+ 'You must specify an `unauthenticated` route when marking a Route as private'
+ )
+ }
+
+ // @MARK: this component intentionally removes all props except children
+ // because the .reduce below will apply props inside out
+ const AuthComponent: React.FC<{ children: ReactNode }> = ({
+ children,
+ }) => {
+ return (
+
+ {children}
+
+ )
+ }
+
+ wrappersWithAuthMaybe = [AuthComponent, ...wrappersWithAuthMaybe]
+ }
+ })
if (wrappersWithAuthMaybe.length > 0) {
// If wrappers exist e.g. [a,b,c] -> and returns a single ReactNode
// Wrap AuthenticatedRoute this way, because if we mutate the wrappers array itself
// it causes full rerenders of the page
return wrappersWithAuthMaybe.reduceRight((acc, wrapper) => {
+ // Merge props from set, the lowest set props will override the higher ones
+ const mergedSetProps = setProps.reduce((acc, props) => {
+ return { ...acc, ...props }
+ }, {})
+
return React.createElement(
wrapper as any,
- {
- ...setProps,
- },
+ mergedSetProps,
acc ? acc : routeLoaderElement
)
}, undefined as ReactNode)
diff --git a/packages/router/src/util.ts b/packages/router/src/util.ts
index 70d28012cb8a..e0bec64c60b6 100644
--- a/packages/router/src/util.ts
+++ b/packages/router/src/util.ts
@@ -464,7 +464,7 @@ interface AnalyzedRoute {
page: PageType | null
redirect: string | null
wrappers: Wrappers
- setProps: Record
+ setProps: Record[]
setId: number
}
@@ -483,7 +483,7 @@ export function analyzeRoutes(
whileLoadingPageFromSet?: WhileLoadingPage
wrappersFromSet?: Wrappers
// we don't know, or care about, what props users are passing down
- propsFromSet?: Record
+ propsFromSet?: Record[]
setId?: number
}
@@ -507,7 +507,7 @@ export function analyzeRoutes(
nodes,
whileLoadingPageFromSet,
wrappersFromSet = [],
- propsFromSet: previousSetProps = {},
+ propsFromSet: previousSetProps = [],
}: RecurseParams) => {
nodes.forEach((node) => {
if (isValidRoute(node)) {
@@ -612,8 +612,10 @@ export function analyzeRoutes(
// @MARK note unintuitive, but intentional
// You cannot make a nested set public if the parent is private
// i.e. the private prop cannot be overriden by a child Set
+
+ // @TODO: Double check this logic is correct for privatge logic in previous set props
const privateProps =
- isPrivateNode(node) || previousSetProps.private
+ isPrivateNode(node) || previousSetProps.some((props) => props.private)
? { private: true }
: {}
@@ -627,13 +629,15 @@ export function analyzeRoutes(
whileLoadingPageFromCurrentSet || whileLoadingPageFromSet,
setId,
wrappersFromSet: [...wrappersFromSet, ...wrapperComponentsArray],
- propsFromSet: {
+ propsFromSet: [
...previousSetProps,
- // Current one takes precedence
- ...otherPropsFromCurrentSet,
- // See comment at definiion, intenionally at the end
- ...privateProps,
- },
+ {
+ // Current one takes precedence
+ ...otherPropsFromCurrentSet,
+ // See comment at definiion, intenionally at the end
+ ...privateProps,
+ },
+ ],
})
}
}