Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router): Prevent rerendering authenticated routes on hash change #9007

Merged
merged 4 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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