fix TypeError edge-case for parallel slots rendered multiple times (#…

…64271) ### What When rendering a parallel slot multiple times in a single layout, in conjunction with using an error boundary, the following TypeError is thrown: > Cannot destructure property 'parallelRouterKey' of 'param' as it is null ### Why I'm not 100% sure of the reason, but I believe this is because of how React attempts to dededupe (more specifically, "detriplficate") objects that it sees getting passed across the RSC -> client component boundary (and an error boundary is necessarily a client component). When React sees the same object twice, it'll create a reference to that object and then use that reference in future places where it sees the object. My assumption is that there's a bug somewhere here, as the `LayoutRouter` component for the subsequent duplicated parallel slots (after the first one) have no props, hence the TypeError. ### How Rather than passing the error component as a prop to `LayoutRouter`, this puts it as part of the `CacheNodeSeedData` data structure. This is more aligned with other properties anyway (such as `loading` and `rsc` for each segment), and seems to work around this bug as the `initialSeedData` prop is only passed from RSC->client once. EDIT: Confirmed this is also fixed after syncing the latest React, due to facebook/react#28669 Fixes #58485 Closes NEXT-2095
vercel · Apr 17, 2024 · f93ae7c · f93ae7c
1 parent ddf2af5
commit f93ae7c
Show file tree

Hide file tree

Showing 28 changed files with 218 additions and 23 deletions.
diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx
@@ -187,6 +187,7 @@ export function createEmptyCacheNode(): CacheNode {
     parallelRoutes: new Map(),
     lazyDataResolved: false,
     loading: null,
+    error: null,
   }
 }
 
@@ -626,8 +627,9 @@ function Router({
       // Provided in AppTreeContext to ensure it can be overwritten in layout-router
       url: canonicalUrl,
       loading: cache.loading,
+      error: cache.error,
     }
-  }, [cache.parallelRoutes, tree, canonicalUrl, cache.loading])
+  }, [cache.parallelRoutes, tree, canonicalUrl, cache.loading, cache.error])
 
   const globalLayoutRouterContext = useMemo(() => {
     return {

diff --git a/packages/next/src/client/components/layout-router.tsx b/packages/next/src/client/components/layout-router.tsx
@@ -9,7 +9,6 @@ import type {
   FlightSegmentPath,
   Segment,
 } from '../../server/app-render/types'
-import type { ErrorComponent } from './error-boundary'
 import type { FocusAndScrollRef } from './router-reducer/router-reducer-types'
 
 import React, {
@@ -358,6 +357,7 @@ function InnerLayoutRouter({
       parallelRoutes: new Map(),
       lazyDataResolved: false,
       loading: null,
+      error: null,
     }
 
     /**
@@ -453,6 +453,7 @@ function InnerLayoutRouter({
         // TODO-APP: overriding of url for parallel routes
         url: url,
         loading: childNode.loading,
+        error: childNode.error,
       }}
     >
       {resolvedRsc}
@@ -507,9 +508,6 @@ function LoadingBoundary({
 export default function OuterLayoutRouter({
   parallelRouterKey,
   segmentPath,
-  error,
-  errorStyles,
-  errorScripts,
   templateStyles,
   templateScripts,
   template,
@@ -519,9 +517,6 @@ export default function OuterLayoutRouter({
 }: {
   parallelRouterKey: string
   segmentPath: FlightSegmentPath
-  error: ErrorComponent | undefined
-  errorStyles: React.ReactNode | undefined
-  errorScripts: React.ReactNode | undefined
   templateStyles: React.ReactNode | undefined
   templateScripts: React.ReactNode | undefined
   template: React.ReactNode
@@ -534,7 +529,7 @@ export default function OuterLayoutRouter({
     throw new Error('invariant expected layout router to be mounted')
   }
 
-  const { childNodes, tree, url, loading } = context
+  const { childNodes, tree, url, loading, error } = context
 
   // Get the current parallelRouter cache node
   let childNodesForParallelRouter = childNodes.get(parallelRouterKey)
@@ -580,9 +575,9 @@ export default function OuterLayoutRouter({
             value={
               <ScrollAndFocusHandler segmentPath={segmentPath}>
                 <ErrorBoundary
-                  errorComponent={error}
-                  errorStyles={errorStyles}
-                  errorScripts={errorScripts}
+                  errorComponent={error?.[0]}
+                  errorStyles={error?.[1]}
+                  errorScripts={error?.[2]}
                 >
                   <LoadingBoundary
                     hasLoading={Boolean(loading)}

diff --git a/...next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.test.tsx b/...next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.test.tsx
@@ -21,6 +21,7 @@ describe('clearCacheNodeDataForSegmentPath', () => {
       parallelRoutes: new Map(),
       lazyDataResolved: false,
       loading: null,
+      error: null,
     }
     const existingCache: CacheNode = {
       lazyData: null,
@@ -30,6 +31,7 @@ describe('clearCacheNodeDataForSegmentPath', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -44,6 +46,7 @@ describe('clearCacheNodeDataForSegmentPath', () => {
                 head: null,
                 prefetchHead: null,
                 loading: null,
+                error: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -59,6 +62,7 @@ describe('clearCacheNodeDataForSegmentPath', () => {
                           parallelRoutes: new Map(),
                           lazyDataResolved: false,
                           loading: null,
+                          error: null,
                         },
                       ],
                     ]),
@@ -75,20 +79,23 @@ describe('clearCacheNodeDataForSegmentPath', () => {
 
     expect(cache).toMatchInlineSnapshot(`
       {
+        "error": null,
         "head": null,
         "lazyData": null,
         "lazyDataResolved": false,
         "loading": null,
         "parallelRoutes": Map {
           "children" => Map {
             "linking" => {
+              "error": null,
               "head": null,
               "lazyData": null,
               "lazyDataResolved": false,
               "loading": null,
               "parallelRoutes": Map {
                 "children" => Map {
                   "" => {
+                    "error": null,
                     "head": null,
                     "lazyData": null,
                     "lazyDataResolved": false,
@@ -109,6 +116,7 @@ describe('clearCacheNodeDataForSegmentPath', () => {
               </React.Fragment>,
             },
             "dashboard" => {
+              "error": null,
               "head": null,
               "lazyData": null,
               "lazyDataResolved": false,

diff --git a/packages/next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.ts b/packages/next/src/client/components/router-reducer/clear-cache-node-data-for-segment-path.ts
@@ -44,6 +44,7 @@ export function clearCacheNodeDataForSegmentPath(
         parallelRoutes: new Map(),
         lazyDataResolved: false,
         loading: null,
+        error: null,
       })
     }
     return
@@ -61,6 +62,7 @@ export function clearCacheNodeDataForSegmentPath(
         parallelRoutes: new Map(),
         lazyDataResolved: false,
         loading: null,
+        error: null,
       })
     }
     return
@@ -76,6 +78,7 @@ export function clearCacheNodeDataForSegmentPath(
       parallelRoutes: new Map(childCacheNode.parallelRoutes),
       lazyDataResolved: childCacheNode.lazyDataResolved,
       loading: childCacheNode.loading,
+      error: childCacheNode.error,
     } as CacheNode
     childSegmentMap.set(cacheKey, childCacheNode)
   }

diff --git a/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx b/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx
@@ -37,7 +37,7 @@ describe('createInitialRouterState', () => {
       buildId,
       initialTree,
       initialCanonicalUrl,
-      initialSeedData: ['', {}, children, null],
+      initialSeedData: ['', {}, children, null, null],
       initialParallelRoutes,
       location: new URL('/linking', 'https://localhost') as any,
       initialHead: <title>Test</title>,
@@ -48,7 +48,7 @@ describe('createInitialRouterState', () => {
       buildId,
       initialTree,
       initialCanonicalUrl,
-      initialSeedData: ['', {}, children, null],
+      initialSeedData: ['', {}, children, null, null],
       initialParallelRoutes,
       location: new URL('/linking', 'https://localhost') as any,
       initialHead: <title>Test</title>,
@@ -62,6 +62,7 @@ describe('createInitialRouterState', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -81,6 +82,7 @@ describe('createInitialRouterState', () => {
                           prefetchRsc: null,
                           parallelRoutes: new Map(),
                           loading: null,
+                          error: null,
                           head: <title>Test</title>,
                           prefetchHead: null,
                           lazyDataResolved: false,
@@ -96,6 +98,7 @@ describe('createInitialRouterState', () => {
                 prefetchHead: null,
                 lazyDataResolved: false,
                 loading: null,
+                error: null,
               },
             ],
           ]),

diff --git a/packages/next/src/client/components/router-reducer/create-initial-router-state.ts b/packages/next/src/client/components/router-reducer/create-initial-router-state.ts
@@ -47,6 +47,7 @@ export function createInitialRouterState({
     parallelRoutes: isServer ? new Map() : initialParallelRoutes,
     lazyDataResolved: false,
     loading: initialSeedData[3],
+    error: initialSeedData[4],
   }
 
   const canonicalUrl =

diff --git a/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.test.tsx b/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.test.tsx
@@ -33,6 +33,7 @@ describe('fillCacheWithNewSubtreeData', () => {
       head: null,
       prefetchHead: null,
       loading: null,
+      error: null,
       parallelRoutes: new Map(),
       lazyDataResolved: false,
     }
@@ -44,6 +45,7 @@ describe('fillCacheWithNewSubtreeData', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -58,6 +60,7 @@ describe('fillCacheWithNewSubtreeData', () => {
                 prefetchHead: null,
                 lazyDataResolved: false,
                 loading: null,
+                error: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -72,6 +75,7 @@ describe('fillCacheWithNewSubtreeData', () => {
                           prefetchHead: null,
                           lazyDataResolved: false,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map(),
                         },
                       ],
@@ -104,6 +108,7 @@ describe('fillCacheWithNewSubtreeData', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -118,6 +123,7 @@ describe('fillCacheWithNewSubtreeData', () => {
                 prefetchHead: null,
                 lazyDataResolved: false,
                 loading: null,
+                error: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -133,6 +139,7 @@ describe('fillCacheWithNewSubtreeData', () => {
                           prefetchHead: null,
                           lazyDataResolved: false,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map(),
                         },
                       ],
@@ -144,6 +151,7 @@ describe('fillCacheWithNewSubtreeData', () => {
                           head: null,
                           prefetchHead: null,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map([
                             [
                               'children',

diff --git a/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts b/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts
@@ -49,13 +49,15 @@ export function fillCacheWithNewSubTreeData(
       const seedData: CacheNodeSeedData = flightDataPath[3]
       const rsc = seedData[2]
       const loading = seedData[3]
+      const error = seedData[4]
       childCacheNode = {
         lazyData: null,
         rsc,
         prefetchRsc: null,
         head: null,
         prefetchHead: null,
         loading,
+        error,
         // Ensure segments other than the one we got data for are preserved.
         parallelRoutes: existingChildCacheNode
           ? new Map(existingChildCacheNode.parallelRoutes)
@@ -101,6 +103,7 @@ export function fillCacheWithNewSubTreeData(
       parallelRoutes: new Map(childCacheNode.parallelRoutes),
       lazyDataResolved: false,
       loading: childCacheNode.loading,
+      error: childCacheNode.error,
     } as CacheNode
     childSegmentMap.set(cacheKey, childCacheNode)
   }

diff --git a/...es/next/src/client/components/router-reducer/fill-lazy-items-till-leaf-with-head.test.tsx b/...es/next/src/client/components/router-reducer/fill-lazy-items-till-leaf-with-head.test.tsx
@@ -44,6 +44,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
       parallelRoutes: new Map(),
       lazyDataResolved: false,
       loading: null,
+      error: null,
     }
     const existingCache: CacheNode = {
       lazyData: null,
@@ -53,6 +54,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -67,6 +69,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                 prefetchHead: null,
                 lazyDataResolved: false,
                 loading: null,
+                error: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -81,6 +84,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                           prefetchHead: null,
                           lazyDataResolved: false,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map(),
                         },
                       ],
@@ -119,6 +123,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
       prefetchHead: null,
       lazyDataResolved: false,
       loading: null,
+      error: null,
       parallelRoutes: new Map([
         [
           'children',
@@ -133,6 +138,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                 prefetchHead: null,
                 lazyDataResolved: false,
                 loading: null,
+                error: null,
                 parallelRoutes: new Map([
                   [
                     'children',
@@ -143,6 +149,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                           lazyData: null,
                           lazyDataResolved: false,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map([
                             [
                               'children',
@@ -155,6 +162,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                                     prefetchRsc: null,
                                     prefetchHead: null,
                                     loading: null,
+                                    error: null,
                                     parallelRoutes: new Map(),
                                     lazyDataResolved: false,
                                     head: (
@@ -182,6 +190,7 @@ describe('fillLazyItemsTillLeafWithHead', () => {
                           head: null,
                           prefetchHead: null,
                           loading: null,
+                          error: null,
                           parallelRoutes: new Map(),
                           lazyDataResolved: false,
                         },