Merge branch 'dev' into markdalgleish/routes-ts

.changeset/grumpy-llamas-leave.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Update externally-accessed resource routes warning to cover null usage as well

.changeset/twenty-waves-cheer.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+Fix `defaultShouldRevalidate` value when using single fetch

integration/single-fetch-test.ts

@@ -1687,12 +1687,46 @@ test.describe("single-fetch", () => {
     });
 
     expect(warnLogs).toEqual([
-      "⚠️ REMIX FUTURE CHANGE: Resource routes will no longer be able to return " +
-        "raw JavaScript objects in v3 when Single Fetch becomes the default. You " +
-        "can prepare for this change at your convenience by wrapping the data " +
-        "returned from your `loader` function in the `routes/resource` route with " +
-        "`json()`.  For instructions on making this change see " +
-        "https://remix.run/docs/en/v2.9.2/guides/single-fetch#resource-routes",
+      "⚠️ REMIX FUTURE CHANGE: Externally-accessed resource routes will no longer be " +
+        "able to return raw JavaScript objects or `null` in React Router v7 when " +
+        "Single Fetch becomes the default. You can prepare for this change at your " +
+        `convenience by wrapping the data returned from your \`loader\` function in ` +
+        `the \`routes/resource\` route with \`json()\`.  For instructions on making this ` +
+        "change, see https://remix.run/docs/en/v2.13.1/guides/single-fetch#resource-routes",
+    ]);
+    console.warn = oldConsoleWarn;
+  });
+
+  test("wraps resource route 'null' returns in json with a deprecation warning", async () => {
+    let oldConsoleWarn = console.warn;
+    let warnLogs: unknown[] = [];
+    console.warn = (...args) => warnLogs.push(...args);
+
+    let fixture = await createFixture({
+      config: {
+        future: {
+          v3_singleFetch: true,
+        },
+      },
+      files: {
+        ...files,
+        "app/routes/resource.tsx": js`
+          export function loader() {
+            return null;
+          }
+        `,
+      },
+    });
+    let res = await fixture.requestResource("/resource");
+    expect(await res.json()).toEqual(null);
+
+    expect(warnLogs).toEqual([
+      "⚠️ REMIX FUTURE CHANGE: Externally-accessed resource routes will no longer be " +
+        "able to return raw JavaScript objects or `null` in React Router v7 when " +
+        "Single Fetch becomes the default. You can prepare for this change at your " +
+        `convenience by wrapping the data returned from your \`loader\` function in ` +
+        `the \`routes/resource\` route with \`json()\`.  For instructions on making this ` +
+        "change, see https://remix.run/docs/en/v2.13.1/guides/single-fetch#resource-routes",
     ]);
     console.warn = oldConsoleWarn;
   });
@@ -2327,6 +2361,103 @@ test.describe("single-fetch", () => {
       ).toBe(true);
     });
 
+    test("provides the proper defaultShouldRevalidate value", async ({
+      page,
+    }) => {
+      let fixture = await createFixture({
+        config: {
+          future: {
+            v3_singleFetch: true,
+          },
+        },
+        files: {
+          ...files,
+          "app/routes/_index.tsx": js`
+            import { Link } from '@remix-run/react';
+            export default function Component() {
+              return <Link to="/parent/a">Go to /parent/a</Link>;
+            }
+          `,
+          "app/routes/parent.tsx": js`
+            import { Link, Outlet, useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export function shouldRevalidate({ defaultShouldRevalidate }) {
+              return defaultShouldRevalidate;
+            }
+            export default function Component() {
+              return (
+                <>
+                  <p id="parent">Parent Count: {useLoaderData().count}</p>
+                  <Link to="/parent/a">Go to /parent/a</Link>
+                  <Link to="/parent/b">Go to /parent/b</Link>
+                  <Outlet/>
+                </>
+              );
+            }
+          `,
+          "app/routes/parent.a.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="a">A Count: {useLoaderData().count}</p>;
+            }
+          `,
+          "app/routes/parent.b.tsx": js`
+            import { useLoaderData } from '@remix-run/react';
+            let count = 0;
+            export function loader({ request }) {
+              return { count: ++count };
+            }
+            export default function Component() {
+              return <p id="b">B Count: {useLoaderData().count}</p>;
+            }
+          `,
+        },
+      });
+      let appFixture = await createAppFixture(fixture);
+      let app = new PlaywrightFixture(appFixture, page);
+
+      let urls: string[] = [];
+      page.on("request", (req) => {
+        if (req.url().includes(".data")) {
+          urls.push(req.url());
+        }
+      });
+
+      await app.goto("/");
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 1");
+      expect(await app.getHtml("#a")).toContain("A Count: 1");
+      expect(urls.length).toBe(1);
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/b");
+      await page.waitForSelector("#b");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 2");
+      expect(await app.getHtml("#b")).toContain("B Count: 1");
+      expect(urls.length).toBe(1);
+      // Reload the parent route
+      expect(urls[0].endsWith("/parent/b.data")).toBe(true);
+      urls = [];
+
+      await app.clickLink("/parent/a");
+      await page.waitForSelector("#a");
+      expect(await app.getHtml("#parent")).toContain("Parent Count: 3");
+      expect(await app.getHtml("#a")).toContain("A Count: 2");
+      expect(urls.length).toBe(1);
+      // Reload the parent route
+      expect(urls[0].endsWith("/parent/a.data")).toBe(true);
+    });
+
     test("does not add a _routes param for routes without loaders", async ({
       page,
     }) => {

packages/remix-react/components.tsx

@@ -459,9 +459,10 @@ function PrefetchPageLinksImpl({
         matches,
         manifest,
         location,
+        future,
         "data"
       ),
-    [page, nextMatches, matches, manifest, location]
+    [page, nextMatches, matches, manifest, location, future]
   );
 
   let dataHrefs = React.useMemo(() => {
@@ -535,9 +536,10 @@ function PrefetchPageLinksImpl({
         matches,
         manifest,
         location,
+        future,
         "assets"
       ),
-    [page, nextMatches, matches, manifest, location]
+    [page, nextMatches, matches, manifest, location, future]
   );
 
   let moduleHrefs = React.useMemo(

packages/remix-react/links.ts

@@ -2,7 +2,7 @@ import type { AgnosticDataRouteMatch } from "@remix-run/router";
 import type { Location } from "react-router-dom";
 import { parsePath } from "react-router-dom";
 
-import type { AssetsManifest } from "./entry";
+import type { AssetsManifest, FutureConfig } from "./entry";
 import type { RouteModules, RouteModule } from "./routeModules";
 import type { EntryRoute } from "./routes";
 import { loadRouteModule } from "./routeModules";
@@ -353,6 +353,7 @@ export function getNewMatchesForLinks(
   currentMatches: AgnosticDataRouteMatch[],
   manifest: AssetsManifest,
   location: Location,
+  future: FutureConfig,
   mode: "data" | "assets"
 ): AgnosticDataRouteMatch[] {
   let path = parsePathPatch(page);
@@ -376,7 +377,8 @@ export function getNewMatchesForLinks(
   // NOTE: keep this mostly up-to-date w/ the transition data diff, but this
   // version doesn't care about submissions
   let newMatches =
-    mode === "data" && location.search !== path.search
+    mode === "data" &&
+    (future.v3_singleFetch || location.search !== path.search)
       ? // this is really similar to stuff in transition.ts, maybe somebody smarter
         // than me (or in less of a hurry) can share some of it. You're the best.
         nextMatches.filter((match, index) => {
@@ -389,6 +391,12 @@ export function getNewMatchesForLinks(
             return true;
           }
 
+          // For reused routes on GET navigations, by default:
+          // - Single fetch always revalidates
+          // - Multi fetch revalidates if search params changed
+          let defaultShouldRevalidate =
+            future.v3_singleFetch || location.search !== path.search;
+
           if (match.route.shouldRevalidate) {
             let routeChoice = match.route.shouldRevalidate({
               currentUrl: new URL(
@@ -398,13 +406,13 @@ export function getNewMatchesForLinks(
               currentParams: currentMatches[0]?.params || {},
               nextUrl: new URL(page, window.origin),
               nextParams: match.params,
-              defaultShouldRevalidate: true,
+              defaultShouldRevalidate,
             });
             if (typeof routeChoice === "boolean") {
               return routeChoice;
             }
           }
-          return true;
+          return defaultShouldRevalidate;
         })
       : nextMatches.filter((match, index) => {
           let manifestRoute = manifest.routes[match.route.id];

packages/remix-react/routes.tsx

@@ -1,5 +1,8 @@
 import * as React from "react";
-import type { HydrationState } from "@remix-run/router";
+import type {
+  HydrationState,
+  ShouldRevalidateFunctionArgs,
+} from "@remix-run/router";
 import { UNSAFE_ErrorResponseImpl as ErrorResponse } from "@remix-run/router";
 import type {
   ActionFunctionArgs,
@@ -315,13 +318,12 @@ export function createClientRoutes(
         ...dataRoute,
         ...getRouteComponents(route, routeModule, isSpaMode),
         handle: routeModule.handle,
-        shouldRevalidate: needsRevalidation
-          ? wrapShouldRevalidateForHdr(
-              route.id,
-              routeModule.shouldRevalidate,
-              needsRevalidation
-            )
-          : routeModule.shouldRevalidate,
+        shouldRevalidate: getShouldRevalidateFunction(
+          future,
+          routeModule,
+          route.id,
+          needsRevalidation
+        ),
       });
 
       let initialData = initialState?.loaderData?.[route.id];
@@ -474,19 +476,16 @@ export function createClientRoutes(
             });
         }
 
-        if (needsRevalidation) {
-          lazyRoute.shouldRevalidate = wrapShouldRevalidateForHdr(
-            route.id,
-            mod.shouldRevalidate,
-            needsRevalidation
-          );
-        }
-
         return {
           ...(lazyRoute.loader ? { loader: lazyRoute.loader } : {}),
           ...(lazyRoute.action ? { action: lazyRoute.action } : {}),
           hasErrorBoundary: lazyRoute.hasErrorBoundary,
-          shouldRevalidate: lazyRoute.shouldRevalidate,
+          shouldRevalidate: getShouldRevalidateFunction(
+            future,
+            lazyRoute,
+            route.id,
+            needsRevalidation
+          ),
           handle: lazyRoute.handle,
           // No need to wrap these in layout since the root route is never
           // loaded via route.lazy()
@@ -511,6 +510,32 @@ export function createClientRoutes(
   });
 }
 
+function getShouldRevalidateFunction(
+  future: FutureConfig,
+  route: Partial<DataRouteObject>,
+  routeId: string,
+  needsRevalidation: Set<string> | undefined
+) {
+  // During HDR we force revalidation for updated routes
+  if (needsRevalidation) {
+    return wrapShouldRevalidateForHdr(
+      routeId,
+      route.shouldRevalidate,
+      needsRevalidation
+    );
+  }
+
+  // Single fetch revalidates by default, so override the RR default value which
+  // matches the multi-fetch behavior with `true`
+  if (future.v3_singleFetch && route.shouldRevalidate) {
+    let fn = route.shouldRevalidate;
+    return (opts: ShouldRevalidateFunctionArgs) =>
+      fn({ ...opts, defaultShouldRevalidate: true });
+  }
+
+  return route.shouldRevalidate;
+}
+
 // When an HMR / HDR update happens we opt out of all user-defined
 // revalidation logic and force a revalidation on the first call
 function wrapShouldRevalidateForHdr(

packages/remix-server-runtime/deprecations.ts

@@ -3,11 +3,11 @@ export function resourceRouteJsonWarning(
   routeId: string
 ) {
   return (
-    "⚠️ REMIX FUTURE CHANGE: Resource routes will no longer be able to " +
-    "return raw JavaScript objects in v3 when Single Fetch becomes the default. " +
-    "You can prepare for this change at your convenience by wrapping the data " +
-    `returned from your \`${type}\` function in the \`${routeId}\` route with ` +
-    "`json()`.  For instructions on making this change see " +
-    "https://remix.run/docs/en/v2.9.2/guides/single-fetch#resource-routes"
+    "⚠️ REMIX FUTURE CHANGE: Externally-accessed resource routes will no longer be " +
+    "able to return raw JavaScript objects or `null` in React Router v7 when " +
+    "Single Fetch becomes the default. You can prepare for this change at your " +
+    `convenience by wrapping the data returned from your \`${type}\` function in ` +
+    `the \`${routeId}\` route with \`json()\`.  For instructions on making this ` +
+    "change, see https://remix.run/docs/en/v2.13.1/guides/single-fetch#resource-routes"
   );
 }