From df797835c2fe0c4160c377314535a60bb7841d92 Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Tue, 28 Feb 2023 09:50:16 -0800 Subject: [PATCH 1/5] fix useMatches with memo --- contributors.yml | 1 + packages/remix-react/components.tsx | 26 ++++++++++++++------------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/contributors.yml b/contributors.yml index fe99dfc30d3..df44992ebb3 100644 --- a/contributors.yml +++ b/contributors.yml @@ -482,6 +482,7 @@ - weavdale - willhack - willin +- wizardlyhel - wKovacs64 - wladiston - wtlin1228 diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 092d4e52e6f..1643ed41829 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -1133,18 +1133,20 @@ export interface RouteMatch { export function useMatches(): RouteMatch[] { let { routeModules } = useRemixContext(); let matches = useMatchesRR(); - return matches.map((match) => { - let remixMatch: RouteMatch = { - id: match.id, - pathname: match.pathname, - params: match.params, - data: match.data, - // Need to grab handle here since we don't have it at client-side route - // creation time - handle: routeModules[match.id].handle, - }; - return remixMatch; - }); + return React.useMemo(() => { + return matches.map((match) => { + let remixMatch: RouteMatch = { + id: match.id, + pathname: match.pathname, + params: match.params, + data: match.data, + // Need to grab handle here since we don't have it at client-side route + // creation time + handle: routeModules[match.id].handle, + }; + return remixMatch; + }); + }, [matches, routeModules]); } /** From a493621eaee5beb5d72f1873061bdb3a7a970a74 Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Tue, 28 Feb 2023 09:52:38 -0800 Subject: [PATCH 2/5] Create dirty-wolves-knock.md --- .changeset/dirty-wolves-knock.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/dirty-wolves-knock.md diff --git a/.changeset/dirty-wolves-knock.md b/.changeset/dirty-wolves-knock.md new file mode 100644 index 00000000000..5ee4a8e81ce --- /dev/null +++ b/.changeset/dirty-wolves-knock.md @@ -0,0 +1,6 @@ +--- +"remix": patch +"@remix-run/react": patch +--- + +Fix useMatches returning different loader data on sub navigation on client side From bd03a1ed6a9cb12d575a0412042f11c8cd67f745 Mon Sep 17 00:00:00 2001 From: Helen Lin Date: Tue, 28 Feb 2023 10:12:46 -0800 Subject: [PATCH 3/5] simplfy return --- packages/remix-react/components.tsx | 30 +++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 1643ed41829..e05cce6d42d 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -1133,20 +1133,22 @@ export interface RouteMatch { export function useMatches(): RouteMatch[] { let { routeModules } = useRemixContext(); let matches = useMatchesRR(); - return React.useMemo(() => { - return matches.map((match) => { - let remixMatch: RouteMatch = { - id: match.id, - pathname: match.pathname, - params: match.params, - data: match.data, - // Need to grab handle here since we don't have it at client-side route - // creation time - handle: routeModules[match.id].handle, - }; - return remixMatch; - }); - }, [matches, routeModules]); + return React.useMemo( + () => + matches.map((match) => { + let remixMatch: RouteMatch = { + id: match.id, + pathname: match.pathname, + params: match.params, + data: match.data, + // Need to grab handle here since we don't have it at client-side route + // creation time + handle: routeModules[match.id].handle, + }; + return remixMatch; + }), + [matches, routeModules] + ); } /** From 9238abe0c05aa967d146710ba818c731fe9a8188 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 1 Mar 2023 10:40:22 -0500 Subject: [PATCH 4/5] Add integration test --- .changeset/dirty-wolves-knock.md | 2 +- integration/matches-test.ts | 196 +++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 integration/matches-test.ts diff --git a/.changeset/dirty-wolves-knock.md b/.changeset/dirty-wolves-knock.md index 5ee4a8e81ce..1f2b3578b2b 100644 --- a/.changeset/dirty-wolves-knock.md +++ b/.changeset/dirty-wolves-knock.md @@ -3,4 +3,4 @@ "@remix-run/react": patch --- -Fix useMatches returning different loader data on sub navigation on client side +Memoize `useMatches` in the Remix layer diff --git a/integration/matches-test.ts b/integration/matches-test.ts new file mode 100644 index 00000000000..8286216ab31 --- /dev/null +++ b/integration/matches-test.ts @@ -0,0 +1,196 @@ +import { test, expect } from "@playwright/test"; + +import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; +import type { Fixture, AppFixture } from "./helpers/create-fixture"; +import { PlaywrightFixture } from "./helpers/playwright-fixture"; + +test.describe("useMatches", () => { + let fixture: Fixture; + let appFixture: AppFixture; + + test.beforeAll(async () => { + fixture = await createFixture({ + files: { + "app/root.jsx": js` + import * as React from 'react'; + import { json } from "@remix-run/node"; + import { Link, Links, Meta, Outlet, Scripts, useMatches } from "@remix-run/react"; + export const handle = { stuff: "root handle"}; + export const loader = () => json("ROOT"); + export default function Root() { + let matches = useMatches(); + let [matchesCount, setMatchesCount] = React.useState(0); + React.useEffect(() => setMatchesCount(matchesCount + 1), [matches]); + + return ( + + + + + + + About + 
+                    {JSON.stringify(matches, null, 2)}
+
+ {matchesCount > 0 ? 
{matchesCount}
: null} + + + + + ); + } + `, + + "app/routes/index.jsx": js` + import { json } from "@remix-run/node"; + export const handle = { stuff: "index handle"}; + export const loader = () => json("INDEX"); + export default function Index() { + return

Index Page

+ } + `, + + "app/routes/about.jsx": js` + import { json } from "@remix-run/node"; + export const handle = { stuff: "about handle"}; + export const loader = async () => { + await new Promise(r => setTimeout(r, 100)); + return json("ABOUT"); + } + export default function About() { + return

About Page

+ } + `, + + "app/routes/count.jsx": js` + import * as React from 'react'; + import { useMatches } from "@remix-run/react"; + export default function Count() { + let matches = useMatches(); + let [count, setCount] = React.useState(0); + let [matchesCount, setMatchesCount] = React.useState(0); + React.useEffect(() => setMatchesCount(matchesCount + 1), [matches]); + return ( + <> +

Count Page

+ + 
{count}
+ {matchesCount > 0 ? 
{matchesCount}
: null} + + ); + } + `, + }, + }); + + appFixture = await createAppFixture(fixture); + }); + + test.afterAll(() => { + appFixture.close(); + }); + + test("grabs the handle from the route module cache", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/"); + + // Wait for effect + await page.waitForSelector("#matches-count-root"); + expect(await app.getHtml("#matches-count-root")).toMatch(">1<"); + expect(await app.getHtml()).toMatch("Index Page"); + expect(await app.getHtml("#matches")).toEqual(`
+[
+  {
+    "id": "root",
+    "pathname": "/",
+    "params": {},
+    "data": "ROOT",
+    "handle": {
+      "stuff": "root handle"
+    }
+  },
+  {
+    "id": "routes/index",
+    "pathname": "/",
+    "params": {},
+    "data": "INDEX",
+    "handle": {
+      "stuff": "index handle"
+    }
+  }
+]
`); + + // Click and don't wait so we can assert _during_ the navigation that we're + // still showing the index matches and we haven't triggered a new effect + await app.clickLink("/about", { wait: false }); + expect(await app.getHtml("#matches")).toEqual(`
+[
+  {
+    "id": "root",
+    "pathname": "/",
+    "params": {},
+    "data": "ROOT",
+    "handle": {
+      "stuff": "root handle"
+    }
+  },
+  {
+    "id": "routes/index",
+    "pathname": "/",
+    "params": {},
+    "data": "INDEX",
+    "handle": {
+      "stuff": "index handle"
+    }
+  }
+]
`); + expect(await app.getHtml("#matches-count-root")).toMatch(">1<"); + + // Once the new page shows up we should get update dmatches and a single + // new effect execution + await page.waitForSelector("#about"); + expect(await app.getHtml()).toMatch("About Page"); + expect(await app.getHtml("#matches-count-root")).toMatch(">2<"); + expect(await app.getHtml("#matches")).toEqual(`
+[
+  {
+    "id": "root",
+    "pathname": "/",
+    "params": {},
+    "data": "ROOT",
+    "handle": {
+      "stuff": "root handle"
+    }
+  },
+  {
+    "id": "routes/about",
+    "pathname": "/about",
+    "params": {},
+    "data": "ABOUT",
+    "handle": {
+      "stuff": "about handle"
+    }
+  }
+]
`); + }); + + test("memoizes matches from react router", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/count"); + await page.waitForSelector("#matches-count-child"); + expect(await app.getHtml("#count")).toMatch(">0<"); + expect(await app.getHtml("#matches-count-child")).toMatch(">1<"); + await app.clickElement("#increment"); + expect(await app.getHtml("#count")).toMatch(">1<"); + expect(await app.getHtml("#matches-count-child")).toMatch(">1<"); + await app.clickElement("#increment"); + expect(await app.getHtml("#count")).toMatch(">2<"); + expect(await app.getHtml("#matches-count-child")).toMatch(">1<"); + }); +}); From 4a178d1326cd57ce3142e0fa956116195b4cb6dc Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 1 Mar 2023 10:41:43 -0500 Subject: [PATCH 5/5] Remove remix from changeset --- .changeset/dirty-wolves-knock.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/dirty-wolves-knock.md b/.changeset/dirty-wolves-knock.md index 1f2b3578b2b..bf80026a5ae 100644 --- a/.changeset/dirty-wolves-knock.md +++ b/.changeset/dirty-wolves-knock.md @@ -1,5 +1,4 @@ --- -"remix": patch "@remix-run/react": patch ---