Skip to content

Commit

Permalink
fix(router): Prevent rerendering authenticated routes on hash change (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dac09 authored Aug 4, 2023
1 parent f1011c9 commit 0ca221c
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 12 deletions.
5 changes: 3 additions & 2 deletions packages/router/src/AuthenticatedRoute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ interface AuthenticatedRouteProps {
whileLoadingAuth?: () => React.ReactElement | null
private?: boolean
}

export function AuthenticatedRoute(props: AuthenticatedRouteProps) {
export const AuthenticatedRoute: React.FC<AuthenticatedRouteProps> = (
props
) => {
const {
private: isPrivate,
unauthenticated,
Expand Down
11 changes: 9 additions & 2 deletions packages/router/src/__tests__/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
Route,
Private,
Redirect,
routes,
routes as generatedRoutes,
Link,
navigate,
back,
Expand All @@ -45,7 +45,7 @@ import {
import { useLocation } from '../location'
import { useParams } from '../params'
import { Set } from '../Set'
import { Spec } from '../util'
import type { Spec, GeneratedRoutesMap } 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 */
Expand All @@ -59,9 +59,16 @@ type UnknownAuthContextInterface = AuthContextInterface<
unknown,
unknown,
unknown,
unknown,
unknown,
unknown,
unknown,
unknown
>

// The types are generated in the user's project
const routes = generatedRoutes as GeneratedRoutesMap

function createDummyAuthContextValues(
partial: Partial<UnknownAuthContextInterface>
) {
Expand Down
18 changes: 12 additions & 6 deletions packages/router/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
TrailingSlashesTypes,
validatePath,
} from './util'
import type { Wrappers } from './util'

import type { AvailableRoutes } from './index'

Expand Down Expand Up @@ -203,7 +204,7 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
}

interface WrappedPageProps {
wrappers: ReactNode[]
wrappers: Wrappers
routeLoaderElement: ReactNode
setProps: Record<any, any>
}
Expand All @@ -220,27 +221,32 @@ interface WrappedPageProps {
*/
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
let wrappersWithAuthMaybe = wrappers
if (setProps.private) {
if (!setProps.unauthenticated) {
throw new Error(
'You must specify an `unauthenticated` route when marking a Route as private'
)
}

wrappers.unshift(AuthenticatedRoute as any)
wrappersWithAuthMaybe = [AuthenticatedRoute, ...wrappers]
}

if (wrappers.length > 0) {
// If wrappers exist e.g. [a,b,c] -> <a><b><c><routeLoader></c></b></a>
return wrappers.reduceRight((acc, wrapper) => {
if (wrappersWithAuthMaybe.length > 0) {
// If wrappers exist e.g. [a,b,c] -> <a><b><c><routeLoader></c></b></a> 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) => {
return React.createElement(
wrapper as any,
{
...setProps,
},
acc ? acc : routeLoaderElement
)
}, undefined) as any
}, undefined as ReactNode)
}

return routeLoaderElement
Expand Down
5 changes: 3 additions & 2 deletions packages/router/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,14 @@ export type GeneratedRoutesMap = {
}

type RoutePath = string
export type Wrappers = Array<(props: any) => ReactNode>
interface AnalyzedRoute {
path: RoutePath
name: string | null
whileLoadingPage?: WhileLoadingPage
page: PageType | null
redirect: string | null
wrappers: ReactNode[]
wrappers: Wrappers
setProps: Record<any, any>
setId: number
}
Expand All @@ -476,7 +477,7 @@ export function analyzeRoutes(
interface RecurseParams {
nodes: ReturnType<typeof Children.toArray>
whileLoadingPageFromSet?: WhileLoadingPage
wrappersFromSet?: ReactNode[]
wrappersFromSet?: Wrappers
// we don't know, or care about, what props users are passing down
propsFromSet?: Record<string, unknown>
setId?: number
Expand Down

0 comments on commit 0ca221c

Please sign in to comment.