feat: revalidate loaders only when a change to one is detected (#6135)

Author: jacob-ebey

integration/hmr-test.ts

@@ -155,6 +155,9 @@ let fixture = (options: {
 
     "app/routes/_index.tsx": js`
       import { useLoaderData } from "@remix-run/react";
+      export function shouldRevalidate(args) {
+        return args.defaultShouldRevalidate;
+      }
       export default function Index() {
         const t = useLoaderData();
         return (
@@ -219,6 +222,13 @@ test("HMR", async ({ page }) => {
   // uncomment for debugging
   // page.on("console", (msg) => console.log(msg.text()));
   page.on("pageerror", (err) => console.log(err.message));
+  let dataRequests = 0;
+  page.on("request", (request) => {
+    let url = new URL(request.url());
+    if (url.searchParams.has("_data")) {
+      dataRequests++;
+    }
+  });
 
   let portRange = makeRange(3080, 3099);
   let appServerPort = await getPort({ port: portRange });
@@ -276,6 +286,9 @@ test("HMR", async ({ page }) => {
     let newIndex = `
       import { useLoaderData } from "@remix-run/react";
       import styles from "~/styles.module.css";
+      export function shouldRevalidate(args) {
+        return args.defaultShouldRevalidate;
+      }
       export default function Index() {
         const t = useLoaderData();
         return (
@@ -289,6 +302,7 @@ test("HMR", async ({ page }) => {
 
     // detect HMR'd content and style changes
     await page.waitForLoadState("networkidle");
+    
     let h1 = page.getByText("Changed");
     await h1.waitFor({ timeout: HMR_TIMEOUT_MS });
     expect(h1).toHaveCSS("color", "rgb(255, 255, 255)");
@@ -305,13 +319,19 @@ test("HMR", async ({ page }) => {
     expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
     await page.waitForSelector(`#root-counter:has-text("inc 1")`);
 
+    // We should not have done any revalidation yet as only UI has changed
+    expect(dataRequests).toBe(0);
+
     // add loader
     let withLoader1 = `
       import { json } from "@remix-run/node";
       import { useLoaderData } from "@remix-run/react";
 
-      export let loader = () => json({ hello: "world" })
+      export let loader = () => json({ hello: "world" });
 
+      export function shouldRevalidate(args) {
+        return args.defaultShouldRevalidate;
+      }
       export default function Index() {
         let { hello } = useLoaderData<typeof loader>();
         return (
@@ -322,10 +342,14 @@ test("HMR", async ({ page }) => {
       }
     `;
     fs.writeFileSync(indexPath, withLoader1);
+    await page.waitForLoadState("networkidle");
+
     await page.getByText("Hello, world").waitFor({ timeout: HMR_TIMEOUT_MS });
     expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
     await page.waitForSelector(`#root-counter:has-text("inc 1")`);
 
+    expect(dataRequests).toBe(1);
+
     let withLoader2 = `
       import { json } from "@remix-run/node";
       import { useLoaderData } from "@remix-run/react";
@@ -334,6 +358,9 @@ test("HMR", async ({ page }) => {
         return json({ hello: "planet" })
       }
 
+      export function shouldRevalidate(args) {
+        return args.defaultShouldRevalidate;
+      }
       export default function Index() {
         let { hello } = useLoaderData<typeof loader>();
         return (
@@ -344,10 +371,14 @@ test("HMR", async ({ page }) => {
       }
     `;
     fs.writeFileSync(indexPath, withLoader2);
+    await page.waitForLoadState("networkidle");
+
     await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS });
     expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
     await page.waitForSelector(`#root-counter:has-text("inc 1")`);
 
+    expect(dataRequests).toBe(2);
+
     // change shared component
     let updatedCounter = `
       import * as React from "react";
@@ -388,6 +419,10 @@ test("HMR", async ({ page }) => {
     aboutCounter = await page.waitForSelector(
       `#about-counter:has-text("inc 0")`
     );
+    
+    // This should not have triggered any revalidation but our detection is
+    // failing for x-module changes for route module imports
+    // expect(dataRequests).toBe(2);
   } catch (e) {
     console.log("stdout begin -----------------------");
     console.log(devStdout());