From 0ca221cb0312d07388c798fe75bc1a1d101bb786 Mon Sep 17 00:00:00 2001
From: Daniel Choudhury <dannychoudhury@gmail.com>
Date: Fri, 4 Aug 2023 14:25:09 +0700
Subject: [PATCH] fix(router): Prevent rerendering authenticated routes on hash
 change (#9007)

---
 packages/router/src/AuthenticatedRoute.tsx    |  5 +++--
 packages/router/src/__tests__/router.test.tsx | 11 +++++++++--
 packages/router/src/router.tsx                | 18 ++++++++++++------
 packages/router/src/util.ts                   |  5 +++--
 4 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/packages/router/src/AuthenticatedRoute.tsx b/packages/router/src/AuthenticatedRoute.tsx
index dbac615d2446..a3bdba1726d5 100644
--- a/packages/router/src/AuthenticatedRoute.tsx
+++ b/packages/router/src/AuthenticatedRoute.tsx
@@ -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,
diff --git a/packages/router/src/__tests__/router.test.tsx b/packages/router/src/__tests__/router.test.tsx
index b81709992c7e..acb1116fa870 100644
--- a/packages/router/src/__tests__/router.test.tsx
+++ b/packages/router/src/__tests__/router.test.tsx
@@ -36,7 +36,7 @@ import {
   Route,
   Private,
   Redirect,
-  routes,
+  routes as generatedRoutes,
   Link,
   navigate,
   back,
@@ -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 */
@@ -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>
 ) {
diff --git a/packages/router/src/router.tsx b/packages/router/src/router.tsx
index 461a6497966f..e3934e82e088 100644
--- a/packages/router/src/router.tsx
+++ b/packages/router/src/router.tsx
@@ -27,6 +27,7 @@ import {
   TrailingSlashesTypes,
   validatePath,
 } from './util'
+import type { Wrappers } from './util'
 
 import type { AvailableRoutes } from './index'
 
@@ -203,7 +204,7 @@ const LocationAwareRouter: React.FC<RouterProps> = ({
 }
 
 interface WrappedPageProps {
-  wrappers: ReactNode[]
+  wrappers: Wrappers
   routeLoaderElement: ReactNode
   setProps: Record<any, any>
 }
@@ -220,6 +221,9 @@ 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(
@@ -227,12 +231,14 @@ const WrappedPage = memo(
         )
       }
 
-      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,
           {
@@ -240,7 +246,7 @@ const WrappedPage = memo(
           },
           acc ? acc : routeLoaderElement
         )
-      }, undefined) as any
+      }, undefined as ReactNode)
     }
 
     return routeLoaderElement
diff --git a/packages/router/src/util.ts b/packages/router/src/util.ts
index 32081ec7c4b2..ebb2188e2fcf 100644
--- a/packages/router/src/util.ts
+++ b/packages/router/src/util.ts
@@ -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
 }
@@ -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