fix(react-router): preloads (#2945)

* tests(react-router): add test for `preload` flag of `beforeLoad` and `loader` * fix: preloads * fix: simplify preload resolution * fix missing ! --------- Co-authored-by: Manuel Schiller <[email protected]>
TanStack · Dec 7, 2024 · 941ff2d · 941ff2d
1 parent ef99aab
commit 941ff2d
Show file tree

Hide file tree

Showing 2 changed files with 171 additions and 19 deletions.
diff --git a/packages/react-router/src/router.ts b/packages/react-router/src/router.ts
@@ -1743,7 +1743,6 @@ export class Router<
           }
           maskedNext = build(maskedDest)
         }
-        // console.log('buildWithMatches', {foundMask, dest, maskedDest, maskedNext})
       }
 
       const nextMatches = this.getMatchedRoutes(next, dest)
@@ -2118,19 +2117,24 @@ export class Router<
     let updated!: AnyRouteMatch
     const isPending = this.state.pendingMatches?.find((d) => d.id === id)
     const isMatched = this.state.matches.find((d) => d.id === id)
+    const isCached = this.state.cachedMatches.find((d) => d.id === id)
 
     const matchesKey = isPending
       ? 'pendingMatches'
       : isMatched
         ? 'matches'
-        : 'cachedMatches'
+        : isCached
+          ? 'cachedMatches'
+          : ''
 
-    this.__store.setState((s) => ({
-      ...s,
-      [matchesKey]: s[matchesKey]?.map((d) =>
-        d.id === id ? (updated = updater(d)) : d,
-      ),
-    }))
+    if (matchesKey) {
+      this.__store.setState((s) => ({
+        ...s,
+        [matchesKey]: s[matchesKey]?.map((d) =>
+          d.id === id ? (updated = updater(d)) : d,
+        ),
+      }))
+    }
 
     return updated
   }
@@ -2146,7 +2150,7 @@ export class Router<
   loadMatches = async ({
     location,
     matches,
-    preload,
+    preload: allPreload,
     onReady,
     updateMatch = this.updateMatch,
   }: {
@@ -2170,6 +2174,10 @@ export class Router<
       }
     }
 
+    const resolvePreload = (matchId: string) => {
+      return !!(allPreload && !this.state.matches.find((d) => d.id === matchId))
+    }
+
     if (!this.isServer && !this.state.matches.length) {
       triggerOnReady()
     }
@@ -2270,7 +2278,7 @@ export class Router<
               const shouldPending = !!(
                 onReady &&
                 !this.isServer &&
-                !preload &&
+                !resolvePreload(matchId) &&
                 (route.options.loader || route.options.beforeLoad) &&
                 typeof pendingMs === 'number' &&
                 pendingMs !== Infinity &&
@@ -2354,6 +2362,8 @@ export class Router<
                   const { search, params, context, cause } =
                     this.getMatch(matchId)!
 
+                  const preload = resolvePreload(matchId)
+
                   const beforeLoadFnContext: BeforeLoadContextOptions<
                     any,
                     any,
@@ -2364,7 +2374,7 @@ export class Router<
                     search,
                     abortController,
                     params,
-                    preload: !!preload,
+                    preload,
                     context,
                     location,
                     navigate: (opts: any) =>
@@ -2432,7 +2442,9 @@ export class Router<
                 (async () => {
                   const { loaderPromise: prevLoaderPromise } =
                     this.getMatch(matchId)!
+
                   let loaderRunningAsync = false
+
                   if (prevLoaderPromise) {
                     await prevLoaderPromise
                   } else {
@@ -2448,6 +2460,8 @@ export class Router<
                         cause,
                       } = this.getMatch(matchId)!
 
+                      const preload = resolvePreload(matchId)
+
                       return {
                         params,
                         deps: loaderDeps,
@@ -2466,6 +2480,8 @@ export class Router<
                     // This is where all of the stale-while-revalidate magic happens
                     const age = Date.now() - this.getMatch(matchId)!.updatedAt
 
+                    const preload = resolvePreload(matchId)
+
                     const staleAge = preload
                       ? (route.options.preloadStaleTime ??
                         this.options.defaultPreloadStaleTime ??
@@ -2686,7 +2702,7 @@ export class Router<
       await triggerOnReady()
     } catch (err) {
       if (isRedirect(err) || isNotFound(err)) {
-        if (isNotFound(err) && !preload) {
+        if (isNotFound(err) && !allPreload) {
           await triggerOnReady()
         }
         throw err
@@ -2806,17 +2822,21 @@ export class Router<
       dest: opts,
     })
 
-    const loadedMatchIds = Object.fromEntries(
-      [
-        ...this.state.matches,
-        ...(this.state.pendingMatches ?? []),
-        ...this.state.cachedMatches,
-      ].map((d) => [d.id, true]),
+    const activeMatchIds = new Set(
+      [...this.state.matches, ...(this.state.pendingMatches ?? [])].map(
+        (d) => d.id,
+      ),
     )
 
+    const loadedMatchIds = new Set([
+      ...activeMatchIds,
+      ...this.state.cachedMatches.map((d) => d.id),
+    ])
+
+    // If the matches are already loaded, we need to add them to the cachedMatches
     this.__store.batch(() => {
       matches.forEach((match) => {
-        if (!loadedMatchIds[match.id]) {
+        if (!loadedMatchIds.has(match.id)) {
           this.__store.setState((s) => ({
             ...s,
             cachedMatches: [...(s.cachedMatches as any), match],
@@ -2830,6 +2850,14 @@ export class Router<
         matches,
         location: next,
         preload: true,
+        updateMatch: (id, updater) => {
+          // Don't update the match if it's currently loaded
+          if (activeMatchIds.has(id)) {
+            matches = matches.map((d) => (d.id === id ? updater(d) : d))
+          } else {
+            this.updateMatch(id, updater)
+          }
+        },
       })
 
       return matches

diff --git a/packages/react-router/tests/link.test.tsx b/packages/react-router/tests/link.test.tsx
@@ -3794,6 +3794,130 @@ describe('Link', () => {
 
     expect(window.location.pathname).toBe('/posts')
   })
+
+  describe('when preloading a link, `preload` should be', () => {
+    async function runTest({
+      expectedPreload,
+      testIdToHover,
+    }: {
+      expectedPreload: boolean
+      testIdToHover: string
+    }) {
+      const rootRoute = createRootRoute({
+        component: () => {
+          return (
+            <>
+              <Link
+                data-testid="link-1"
+                to="/posts/$postId"
+                params={{ postId: 'id1' }}
+              >
+                To first post
+              </Link>
+              <Link
+                data-testid="link-2"
+                to="/posts/$postId"
+                params={{ postId: 'id2' }}
+              >
+                To second post
+              </Link>
+              <Outlet />
+            </>
+          )
+        },
+      })
+
+      const indexRoute = createRoute({
+        getParentRoute: () => rootRoute,
+        path: '/',
+        component: () => {
+          return (
+            <>
+              <h1>Index</h1>
+            </>
+          )
+        },
+      })
+
+      const PostsComponent = () => {
+        return (
+          <>
+            <h1>Posts</h1>
+            <Outlet />
+          </>
+        )
+      }
+
+      const postsBeforeLoadFn = vi.fn()
+      const postsLoaderFn = vi.fn()
+
+      const postsRoute = createRoute({
+        getParentRoute: () => rootRoute,
+        path: 'posts',
+        component: PostsComponent,
+        beforeLoad: postsBeforeLoadFn,
+        loader: postsLoaderFn,
+      })
+
+      const PostComponent = () => {
+        const params = useParams({ strict: false })
+        return (
+          <>
+            <span>Params: {params.postId}</span>
+          </>
+        )
+      }
+
+      const postBeforeLoadFn = vi.fn()
+      const postLoaderFn = vi.fn()
+
+      const postRoute = createRoute({
+        getParentRoute: () => postsRoute,
+        path: '$postId',
+        component: PostComponent,
+        beforeLoad: postBeforeLoadFn,
+        loader: postLoaderFn,
+      })
+
+      const routeTree = rootRoute.addChildren([
+        indexRoute,
+        postsRoute.addChildren([postRoute]),
+      ])
+
+      const router = createRouter({
+        routeTree,
+        defaultPreload: 'intent',
+      })
+
+      render(<RouterProvider router={router} />)
+      const link = await screen.findByTestId(testIdToHover)
+      fireEvent.mouseOver(link)
+
+      const expected = expect.objectContaining({ preload: expectedPreload })
+      await waitFor(() =>
+        expect(postsBeforeLoadFn).toHaveBeenCalledWith(expected),
+      )
+      await waitFor(() => expect(postsLoaderFn).toHaveBeenCalledWith(expected))
+
+      await waitFor(() =>
+        expect(postBeforeLoadFn).toHaveBeenCalledWith(expected),
+      )
+      await waitFor(() => expect(postLoaderFn).toHaveBeenCalledWith(expected))
+    }
+    test('`true` when on / and hovering `/posts/id1` ', async () => {
+      await runTest({ expectedPreload: true, testIdToHover: 'link-1' })
+    })
+
+    test('`false` when on `/posts/id1` and hovering `/posts/id1`', async () => {
+      window.history.replaceState(null, 'root', '/posts/id1')
+      await runTest({ expectedPreload: false, testIdToHover: 'link-1' })
+    })
+
+    test('`true` when on `/posts/id1` and hovering `/posts/id2`', async () => {
+      window.history.replaceState(null, 'root', '/posts/id1')
+      await runTest({ expectedPreload: false, testIdToHover: 'link-2' })
+    })
+  })
 })
 
 describe('createLink', () => {