From e73c2f923bfbf7ec60053a389afe70f759b977e2 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 4 Aug 2023 15:52:02 +1000 Subject: [PATCH 1/6] fix(react): dedupe prefetch links --- packages/remix-react/components.tsx | 8 ++++++-- packages/remix-react/links.ts | 30 +++++++++++++++++------------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 50f39d1a153..a30044fe724 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -311,6 +311,10 @@ export function composeEventHandlers< }; } +function getHtmlLinkDescriptorKey(link: HtmlLinkDescriptor) { + return link.rel + (link.href || "") + (link.imageSrcSet || ""); +} + /** * Renders the `` tags for the current routes. * @@ -360,7 +364,7 @@ export function Links() { return ( ( // these don't spread `linkProps` because they are full link descriptors // already with their own props - + ))} ); diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index ec4962fb113..ca6e610b784 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -218,7 +218,7 @@ export function getLinksForMatches( .flat(1); let preloads = getCurrentPageModulePreloadHrefs(matches, manifest); - return dedupe(descriptors, preloads); + return dedupeLinkDescriptors(descriptors, preloads); } export async function prefetchStyleLinks( @@ -321,15 +321,17 @@ export async function getStylesheetPrefetchLinks( }) ); - return links - .flat(1) - .filter(isHtmlLinkDescriptor) - .filter((link) => link.rel === "stylesheet" || link.rel === "preload") - .map((link) => - link.rel === "preload" - ? ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) - : ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) - ); + return dedupeLinkDescriptors( + links + .flat(1) + .filter(isHtmlLinkDescriptor) + .filter((link) => link.rel === "stylesheet" || link.rel === "preload") + .map((link) => + link.rel === "preload" + ? ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) + : ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) + ) + ); } // This is ridiculously identical to transition.ts `filterMatchesToLoad` @@ -466,12 +468,16 @@ function dedupeHrefs(hrefs: string[]): string[] { return [...new Set(hrefs)]; } -export function dedupe(descriptors: LinkDescriptor[], preloads: string[]) { +export function dedupeLinkDescriptors( + descriptors: Descriptor[], + preloads?: string[] +) { let set = new Set(); let preloadsSet = new Set(preloads); return descriptors.reduce((deduped, descriptor) => { let alreadyModulePreload = + preloads && !isPageLinkDescriptor(descriptor) && descriptor.as === "script" && descriptor.href && @@ -488,7 +494,7 @@ export function dedupe(descriptors: LinkDescriptor[], preloads: string[]) { } return deduped; - }, [] as LinkDescriptor[]); + }, [] as Descriptor[]); } // https://github.com/remix-run/history/issues/897 From 6f966c1786d8778b2851bc29f8c9b2ba93976a2d Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 7 Aug 2023 14:04:09 +1000 Subject: [PATCH 2/6] sort keys when creating id, add dedupe test --- integration/prefetch-test.ts | 143 +++++++++++++++++++++++++++- packages/remix-react/components.tsx | 20 ++-- packages/remix-react/links.ts | 30 ++++-- 3 files changed, 174 insertions(+), 19 deletions(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index fe326c1e280..5c8b4fb8e50 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -1,6 +1,11 @@ import { test, expect } from "@playwright/test"; -import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; +import { + createAppFixture, + createFixture, + js, + css, +} from "./helpers/create-fixture"; import type { Fixture, FixtureInit, @@ -424,4 +429,140 @@ test.describe("other scenarios", () => { ); expect(stylesheets.length).toBe(1); }); + + test("dedupes prefetch tags", async ({ page }) => { + fixture = await createFixture({ + files: { + "app/root.tsx": js` + import { + Link, + Links, + Meta, + Outlet, + Scripts, + useLoaderData, + } from "@remix-run/react"; + + export default function Root() { + const styles = + 'a:hover { color: red; } a:hover:after { content: " (hovered)"; }' + + 'a:focus { color: green; } a:focus:after { content: " (focused)"; }'; + + return ( + + + + + + + +

Root

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

Index

; + } + `, + + "app/routes/with-nested-links.tsx": js` + import { Outlet } from "@remix-run/react"; + import globalCss from "../global.css"; + + export function links() { + return [ + // Same links as child route but with different key order + { + rel: "stylesheet", + href: globalCss, + }, + { + rel: "preload", + as: "image", + imageSrcSet: "image-600.jpg 600w, image-1200.jpg 1200w", + imageSizes: "9999px", + }, + ]; + } + export default function() { + return ; + } + `, + + "app/routes/with-nested-links.nested.tsx": js` + import globalCss from '../global.css'; + import localCss from '../local.css'; + + export function links() { + return [ + // Same links as parent route but with different key order + { + href: globalCss, + rel: "stylesheet", + }, + { + imageSrcSet: "image-600.jpg 600w, image-1200.jpg 1200w", + imageSizes: "9999px", + rel: "preload", + as: "image", + }, + // Unique links for child route + { + rel: "stylesheet", + href: localCss, + }, + { + rel: "preload", + as: "image", + imageSrcSet: "image-700.jpg 700w, image-1400.jpg 1400w", + imageSizes: "9999px", + }, + ]; + } + export default function() { + return

With Nested Links

; + } + `, + }, + }); + appFixture = await createAppFixture(fixture); + + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + await page.hover("a[href='/with-nested-links/nested']"); + await page.waitForSelector("#nav link[rel='prefetch'][as='style']", { + state: "attached", + }); + expect( + await page.locator("#nav link[rel='prefetch'][as='style']").count() + ).toBe(2); + expect( + await page.locator("#nav link[rel='prefetch'][as='image']").count() + ).toBe(2); + }); }); diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index a30044fe724..c00bcd89f3a 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -39,7 +39,7 @@ import { getLinksForMatches, getModuleLinkHrefs, getNewMatchesForLinks, - getStylesheetPrefetchLinks, + getPrefetchLinks, isPageLinkDescriptor, } from "./links"; import type { HtmlLinkDescriptor, PrefetchPageDescriptor } from "./links"; @@ -406,26 +406,24 @@ export function PrefetchPageLinks({ ); } -function usePrefetchedStylesheets(matches: AgnosticDataRouteMatch[]) { +function usePrefetchLinks(matches: AgnosticDataRouteMatch[]) { let { manifest, routeModules } = useRemixContext(); - let [styleLinks, setStyleLinks] = React.useState([]); + let [links, setLinks] = React.useState([]); React.useEffect(() => { let interrupted: boolean = false; - getStylesheetPrefetchLinks(matches, manifest, routeModules).then( - (links) => { - if (!interrupted) setStyleLinks(links); - } - ); + getPrefetchLinks(matches, manifest, routeModules).then((links) => { + if (!interrupted) setLinks(links); + }); return () => { interrupted = true; }; }, [matches, manifest, routeModules]); - return styleLinks; + return links; } function PrefetchPageLinksImpl({ @@ -477,7 +475,7 @@ function PrefetchPageLinksImpl({ // needs to be a hook with async behavior because we need the modules, not // just the manifest like the other links in here. - let styleLinks = usePrefetchedStylesheets(newMatchesForAssets); + let prefetchLinks = usePrefetchLinks(newMatchesForAssets); return ( <> @@ -487,7 +485,7 @@ function PrefetchPageLinksImpl({ {moduleHrefs.map((href) => ( ))} - {styleLinks.map((link) => ( + {prefetchLinks.map((link) => ( // these don't spread `linkProps` because they are full link descriptors // already with their own props diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index ca6e610b784..f7c637a6d14 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -306,7 +306,7 @@ function isHtmlLinkDescriptor(object: any): object is HtmlLinkDescriptor { return typeof object.rel === "string" && typeof object.href === "string"; } -export async function getStylesheetPrefetchLinks( +export async function getPrefetchLinks( matches: AgnosticDataRouteMatch[], manifest: AssetsManifest, routeModules: RouteModules @@ -327,9 +327,9 @@ export async function getStylesheetPrefetchLinks( .filter(isHtmlLinkDescriptor) .filter((link) => link.rel === "stylesheet" || link.rel === "preload") .map((link) => - link.rel === "preload" - ? ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) - : ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) + link.rel === "stylesheet" + ? ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) + : ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) ) ); } @@ -468,6 +468,22 @@ function dedupeHrefs(hrefs: string[]): string[] { return [...new Set(hrefs)]; } +function sortKeys(obj: Obj): Obj { + let sorted = {} as Obj; + let keys = Object.keys(obj).sort(); + + for (let key of keys) { + sorted[key as keyof Obj] = obj[key as keyof Obj]; + } + + return sorted; +} + +function getLinkDescriptorId(descriptor: LinkDescriptor) { + let sortedDescriptor = sortKeys(descriptor); + return JSON.stringify(sortedDescriptor); +} + export function dedupeLinkDescriptors( descriptors: Descriptor[], preloads?: string[] @@ -487,9 +503,9 @@ export function dedupeLinkDescriptors( return deduped; } - let str = JSON.stringify(descriptor); - if (!set.has(str)) { - set.add(str); + let id = getLinkDescriptorId(descriptor); + if (!set.has(id)) { + set.add(id); deduped.push(descriptor); } From 86dc3a43d9b0504530622e8106de212f5cc9180d Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 7 Aug 2023 14:35:08 +1000 Subject: [PATCH 3/6] use consistent logic for dedupe and keys --- packages/remix-react/components.tsx | 40 ++++++++++++++--------------- packages/remix-react/links.ts | 30 +++++++++++++--------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index c00bcd89f3a..10b6e64a337 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -36,13 +36,13 @@ import { RemixRootDefaultErrorBoundary } from "./errorBoundaries"; import invariant from "./invariant"; import { getDataLinkHrefs, - getLinksForMatches, + getKeyedLinksForMatches, + getKeyedPrefetchLinks, getModuleLinkHrefs, getNewMatchesForLinks, - getPrefetchLinks, isPageLinkDescriptor, } from "./links"; -import type { HtmlLinkDescriptor, PrefetchPageDescriptor } from "./links"; +import type { KeyedHtmlLinkDescriptor, PrefetchPageDescriptor } from "./links"; import { createHtml, escapeHtml } from "./markup"; import type { MetaFunction, @@ -311,10 +311,6 @@ export function composeEventHandlers< }; } -function getHtmlLinkDescriptorKey(link: HtmlLinkDescriptor) { - return link.rel + (link.href || "") + (link.imageSrcSet || ""); -} - /** * Renders the `` tags for the current routes. * @@ -331,16 +327,16 @@ export function Links() { ) : routerMatches; - let links = React.useMemo( - () => getLinksForMatches(matches, routeModules, manifest), + let keyedLinks = React.useMemo( + () => getKeyedLinksForMatches(matches, routeModules, manifest), [matches, routeModules, manifest] ); return ( <> - {links.map((link) => { + {keyedLinks.map(({ key, link }) => { if (isPageLinkDescriptor(link)) { - return ; + return ; } let imageSrcSet: string | null = null; @@ -364,7 +360,7 @@ export function Links() { return ( ([]); + let [keyedPrefetchLinks, setKeyedPrefetchLinks] = React.useState< + KeyedHtmlLinkDescriptor[] + >([]); React.useEffect(() => { let interrupted: boolean = false; - getPrefetchLinks(matches, manifest, routeModules).then((links) => { - if (!interrupted) setLinks(links); + getKeyedPrefetchLinks(matches, manifest, routeModules).then((links) => { + if (!interrupted) { + setKeyedPrefetchLinks(links); + } }); return () => { @@ -423,7 +423,7 @@ function usePrefetchLinks(matches: AgnosticDataRouteMatch[]) { }; }, [matches, manifest, routeModules]); - return links; + return keyedPrefetchLinks; } function PrefetchPageLinksImpl({ @@ -475,7 +475,7 @@ function PrefetchPageLinksImpl({ // needs to be a hook with async behavior because we need the modules, not // just the manifest like the other links in here. - let prefetchLinks = usePrefetchLinks(newMatchesForAssets); + let keyedPrefetchLinks = useKeyedPrefetchLinks(newMatchesForAssets); return ( <> @@ -485,10 +485,10 @@ function PrefetchPageLinksImpl({ {moduleHrefs.map((href) => ( ))} - {prefetchLinks.map((link) => ( + {keyedPrefetchLinks.map(({ key, link }) => ( // these don't spread `linkProps` because they are full link descriptors // already with their own props - + ))} ); diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index f7c637a6d14..f7463eccd01 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -201,15 +201,17 @@ export type LinkDescriptor = HtmlLinkDescriptor | PrefetchPageDescriptor; //////////////////////////////////////////////////////////////////////////////// +type KeyedLinkDescriptor = { key: string; link: LinkDescriptor }; + /** * Gets all the links for a set of matches. The modules are assumed to have been * loaded already. */ -export function getLinksForMatches( +export function getKeyedLinksForMatches( matches: AgnosticDataRouteMatch[], routeModules: RouteModules, manifest: AssetsManifest -): LinkDescriptor[] { +): KeyedLinkDescriptor[] { let descriptors = matches .map((match): LinkDescriptor[] => { let module = routeModules[match.route.id]; @@ -218,7 +220,10 @@ export function getLinksForMatches( .flat(1); let preloads = getCurrentPageModulePreloadHrefs(matches, manifest); - return dedupeLinkDescriptors(descriptors, preloads); + return dedupeLinkDescriptors(descriptors, preloads).map((link) => ({ + key: getLinkDescriptorKey(link), + link, + })); } export async function prefetchStyleLinks( @@ -306,11 +311,13 @@ function isHtmlLinkDescriptor(object: any): object is HtmlLinkDescriptor { return typeof object.rel === "string" && typeof object.href === "string"; } -export async function getPrefetchLinks( +export type KeyedHtmlLinkDescriptor = { key: string; link: HtmlLinkDescriptor }; + +export async function getKeyedPrefetchLinks( matches: AgnosticDataRouteMatch[], manifest: AssetsManifest, routeModules: RouteModules -): Promise { +): Promise { let links = await Promise.all( matches.map(async (match) => { let mod = await loadRouteModule( @@ -331,7 +338,7 @@ export async function getPrefetchLinks( ? ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) : ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) ) - ); + ).map((link) => ({ key: getLinkDescriptorKey(link), link })); } // This is ridiculously identical to transition.ts `filterMatchesToLoad` @@ -479,9 +486,8 @@ function sortKeys(obj: Obj): Obj { return sorted; } -function getLinkDescriptorId(descriptor: LinkDescriptor) { - let sortedDescriptor = sortKeys(descriptor); - return JSON.stringify(sortedDescriptor); +function getLinkDescriptorKey(descriptor: LinkDescriptor) { + return JSON.stringify(sortKeys(descriptor)); } export function dedupeLinkDescriptors( @@ -503,9 +509,9 @@ export function dedupeLinkDescriptors( return deduped; } - let id = getLinkDescriptorId(descriptor); - if (!set.has(id)) { - set.add(id); + let key = getLinkDescriptorKey(descriptor); + if (!set.has(key)) { + set.add(key); deduped.push(descriptor); } From 57d7bb93407b509641f99ed917a2ce7b81f3ff73 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 7 Aug 2023 14:36:05 +1000 Subject: [PATCH 4/6] add changeset --- .changeset/deduplicate-prefetch-link-tags.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/deduplicate-prefetch-link-tags.md diff --git a/.changeset/deduplicate-prefetch-link-tags.md b/.changeset/deduplicate-prefetch-link-tags.md new file mode 100644 index 00000000000..b0b8ad82b6a --- /dev/null +++ b/.changeset/deduplicate-prefetch-link-tags.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +Deduplicate prefetch link tags From 741f0f92398e3d9f7f71c5cd9f38de6c01b7e68d Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 7 Aug 2023 14:44:36 +1000 Subject: [PATCH 5/6] add keys to return value when deduping --- packages/remix-react/links.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index f7463eccd01..8b054e49105 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -201,8 +201,6 @@ export type LinkDescriptor = HtmlLinkDescriptor | PrefetchPageDescriptor; //////////////////////////////////////////////////////////////////////////////// -type KeyedLinkDescriptor = { key: string; link: LinkDescriptor }; - /** * Gets all the links for a set of matches. The modules are assumed to have been * loaded already. @@ -220,10 +218,7 @@ export function getKeyedLinksForMatches( .flat(1); let preloads = getCurrentPageModulePreloadHrefs(matches, manifest); - return dedupeLinkDescriptors(descriptors, preloads).map((link) => ({ - key: getLinkDescriptorKey(link), - link, - })); + return dedupeLinkDescriptors(descriptors, preloads); } export async function prefetchStyleLinks( @@ -338,7 +333,7 @@ export async function getKeyedPrefetchLinks( ? ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor) : ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor) ) - ).map((link) => ({ key: getLinkDescriptorKey(link), link })); + ); } // This is ridiculously identical to transition.ts `filterMatchesToLoad` @@ -490,10 +485,15 @@ function getLinkDescriptorKey(descriptor: LinkDescriptor) { return JSON.stringify(sortKeys(descriptor)); } -export function dedupeLinkDescriptors( +type KeyedLinkDescriptor = { + key: string; + link: Descriptor; +}; + +function dedupeLinkDescriptors( descriptors: Descriptor[], preloads?: string[] -) { +): KeyedLinkDescriptor[] { let set = new Set(); let preloadsSet = new Set(preloads); @@ -512,11 +512,11 @@ export function dedupeLinkDescriptors( let key = getLinkDescriptorKey(descriptor); if (!set.has(key)) { set.add(key); - deduped.push(descriptor); + deduped.push({ key, link: descriptor }); } return deduped; - }, [] as Descriptor[]); + }, [] as KeyedLinkDescriptor[]); } // https://github.com/remix-run/history/issues/897 From 115e25f0a492dd023eebf494e240074b19705296 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Mon, 7 Aug 2023 14:47:21 +1000 Subject: [PATCH 6/6] inline key generation --- packages/remix-react/links.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts index 8b054e49105..5f28146c944 100644 --- a/packages/remix-react/links.ts +++ b/packages/remix-react/links.ts @@ -481,10 +481,6 @@ function sortKeys(obj: Obj): Obj { return sorted; } -function getLinkDescriptorKey(descriptor: LinkDescriptor) { - return JSON.stringify(sortKeys(descriptor)); -} - type KeyedLinkDescriptor = { key: string; link: Descriptor; @@ -509,7 +505,7 @@ function dedupeLinkDescriptors( return deduped; } - let key = getLinkDescriptorKey(descriptor); + let key = JSON.stringify(sortKeys(descriptor)); if (!set.has(key)) { set.add(key); deduped.push({ key, link: descriptor });