From 2503059e33b458140be82fd55149e8473d3b9e7b Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 23 Jun 2023 17:40:37 -0400 Subject: [PATCH 1/4] Avoid re-prefetching stylesheets for active routes during a revalidation --- .changeset/stylesheet-re-prefetch.md | 5 +++++ packages/remix-react/links.ts | 10 ++++++++-- packages/remix-react/routes.tsx | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 .changeset/stylesheet-re-prefetch.md diff --git a/.changeset/stylesheet-re-prefetch.md b/.changeset/stylesheet-re-prefetch.md new file mode 100644 index 00000000000..3310ddc8f28 --- /dev/null +++ b/.changeset/stylesheet-re-prefetch.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +Avoid re-prefetching stylesheets for active routes during a revalidation diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index 6c070d5746d..1d46f6696be 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -239,6 +239,7 @@ export function getLinksForMatches( export async function prefetchStyleLinks( routeModule: RouteModule ): Promise { + debugger; if (!routeModule.links) return; let descriptors = routeModule.links(); if (!descriptors) return; @@ -254,9 +255,14 @@ export async function prefetchStyleLinks( } } - // don't block for non-matching media queries + // don't block for non-matching media queries, or for stylesheets that are + // already in the DOM (active route revalidations) let matchingLinks = styleLinks.filter( - (link) => !link.media || window.matchMedia(link.media).matches + (link) => + (!link.media || window.matchMedia(link.media).matches) && + document.querySelector( + `head link[rel="stylesheet"][href="${link.href}"]` + ) != null ); await Promise.all(matchingLinks.map(prefetchStyleLink)); diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 3e49768e995..49cfe072277 100644 --- a/packages/remix-react/routes.tsx +++ b/packages/remix-react/routes.tsx @@ -203,6 +203,7 @@ async function loadRouteModuleWithBlockingLinks( routeModules: RouteModules ) { let routeModule = await loadRouteModule(route, routeModules); + // Should we not call this is the route is already active? await prefetchStyleLinks(routeModule); return routeModule; } From c87995347b0b9e570848186b7c897a601656d13e Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 30 Jun 2023 13:39:25 -0400 Subject: [PATCH 2/4] Remove debugger --- packages/remix-react/links.ts | 1 - packages/remix-react/routes.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index 1d46f6696be..839f347e2de 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -239,7 +239,6 @@ export function getLinksForMatches( export async function prefetchStyleLinks( routeModule: RouteModule ): Promise { - debugger; if (!routeModule.links) return; let descriptors = routeModule.links(); if (!descriptors) return; diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 49cfe072277..73c08cb89ae 100644 --- a/packages/remix-react/routes.tsx +++ b/packages/remix-react/routes.tsx @@ -203,7 +203,7 @@ async function loadRouteModuleWithBlockingLinks( routeModules: RouteModules ) { let routeModule = await loadRouteModule(route, routeModules); - // Should we not call this is the route is already active? + // TODO: Should we not call this if the route is already active? await prefetchStyleLinks(routeModule); return routeModule; } From 3784ed2e5e6cc3dee2508fe9a5af0b674c95a500 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 5 Jul 2023 16:38:47 -0400 Subject: [PATCH 3/4] Add integration test --- integration/prefetch-test.ts | 94 +++++++++++++++++++++++++++++++++ packages/remix-react/links.ts | 4 +- packages/remix-react/routes.tsx | 1 - 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index c07b00abc6e..b79eaf76cbe 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -342,3 +342,97 @@ test.describe("prefetch=viewport", () => { await expect(page.locator("div link")).toHaveCount(0); }); }); + +test.describe("other scenarios", () => { + let fixture: Fixture; + let appFixture: AppFixture; + + test.afterAll(() => { + appFixture?.close(); + }); + + test("does not add prefetch links for stylesheets already in the DOM (active routes)", async ({ + page, + }) => { + fixture = await createFixture({ + config: { + future: { v2_routeConvention: true }, + }, + files: { + "app/root.jsx": js` + import { Links, Meta, Scripts, useFetcher } from "@remix-run/react"; + import globalCss from "./global.css"; + + export function links() { + return [{ rel: "stylesheet", href: globalCss }]; + } + + export async function action() { + await new Promise(r => setTimeout(r, 100)); + return null; + } + + export async function loader() { + await new Promise(r => setTimeout(r, 100)); + return null; + } + + export default function Root() { + let fetcher = useFetcher(); + + return ( + + + + + + + +

{fetcher.state}

+ + + + ); + } + `, + + "app/global.css": ` + body { + background-color: black; + color: white; + } + `, + + "app/routes/_index.jsx": js` + export default function() { + return

Index

; + } + `, + }, + }); + appFixture = await createAppFixture(fixture); + let requests: { type: string; url: string }[] = []; + + page.on("request", (req) => { + requests.push({ + type: req.resourceType(), + url: req.url(), + }); + }); + + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await page.click("#submit-fetcher"); + await page.waitForSelector("#fetcher-state--idle"); + // We should not send a second request for4 this root stylesheet that's + // already been rendered in the DOM + let stylesheets = requests.filter( + (r) => r.type === "stylesheet" && /\/global-[a-z0-9]+\.css/i.test(r.url) + ); + expect(stylesheets.length).toBe(1); + }); +}); diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index 839f347e2de..1edcd6c5841 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -259,9 +259,7 @@ export async function prefetchStyleLinks( let matchingLinks = styleLinks.filter( (link) => (!link.media || window.matchMedia(link.media).matches) && - document.querySelector( - `head link[rel="stylesheet"][href="${link.href}"]` - ) != null + !document.querySelector(`link[rel="stylesheet"][href="${link.href}"]`) ); await Promise.all(matchingLinks.map(prefetchStyleLink)); diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx index 73c08cb89ae..3e49768e995 100644 --- a/packages/remix-react/routes.tsx +++ b/packages/remix-react/routes.tsx @@ -203,7 +203,6 @@ async function loadRouteModuleWithBlockingLinks( routeModules: RouteModules ) { let routeModule = await loadRouteModule(route, routeModules); - // TODO: Should we not call this if the route is already active? await prefetchStyleLinks(routeModule); return routeModule; } From eca4854937f36c982adb0b55444713344b1dafb6 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 5 Jul 2023 16:45:33 -0400 Subject: [PATCH 4/4] Fix comment and remove delays --- integration/prefetch-test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index b79eaf76cbe..a70c714e10c 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -368,12 +368,10 @@ test.describe("other scenarios", () => { } export async function action() { - await new Promise(r => setTimeout(r, 100)); return null; } export async function loader() { - await new Promise(r => setTimeout(r, 100)); return null; } @@ -428,7 +426,7 @@ test.describe("other scenarios", () => { await app.goto("/"); await page.click("#submit-fetcher"); await page.waitForSelector("#fetcher-state--idle"); - // We should not send a second request for4 this root stylesheet that's + // We should not send a second request for this root stylesheet that's // already been rendered in the DOM let stylesheets = requests.filter( (r) => r.type === "stylesheet" && /\/global-[a-z0-9]+\.css/i.test(r.url)