From c24d0c6dd17961584bb3f9e0cc2d6ba2464b05a1 Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Sat, 22 Feb 2025 16:42:50 -0700 Subject: [PATCH 1/3] Add failing test for Map and Set order When the loader data contains a `Map` or `Set` object, the data maintains the correct order in the server-rendered response. However, once the page hydrates, the order of the items in the `Map` or `Set` are reversed, leading to a hydration error from React. --- integration/bug-report-test.ts | 55 ++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/integration/bug-report-test.ts b/integration/bug-report-test.ts index 8be63c7fc2..1a28328ae4 100644 --- a/integration/bug-report-test.ts +++ b/integration/bug-report-test.ts @@ -64,27 +64,20 @@ test.beforeAll(async () => { // `createFixture` will make an app and run your tests against it. //////////////////////////////////////////////////////////////////////////// files: { - "app/routes/_index.tsx": js` - import { useLoaderData, Link } from "react-router"; - + "app/routes/map.tsx": js` export function loader() { - return "pizza"; + return new Map([[1, 1], [2, 2], [3, 3]]); } - - export default function Index() { - let data = useLoaderData(); - return ( -
- {data} - Other Route -
- ) + export default function Map({ loaderData }) { + return
{JSON.stringify(Array.from(loaderData.entries()))}
; } `, - - "app/routes/burgers.tsx": js` - export default function Index() { - return
cheeseburger
; + "app/routes/set.tsx": js` + export function loader() { + return new Set([1, 2, 3]); + } + export default function Set({ loaderData }) { + return
{JSON.stringify(Array.from(loaderData.values()))}
; } `, }, @@ -103,16 +96,19 @@ test.afterAll(() => { // add a good description for what you expect React Router to do 👇🏽 //////////////////////////////////////////////////////////////////////////////// -test("[description of what you expect it to do]", async ({ page }) => { +test("Maintains correct order of Map objects when hydrating", async ({ + page, +}) => { let app = new PlaywrightFixture(appFixture, page); // You can test any request your app might get using `fixture`. - let response = await fixture.requestDocument("/"); - expect(await response.text()).toMatch("pizza"); + let response = await fixture.requestDocument("/map"); + expect(await response.text()).toMatch("[[1,1],[2,2],[3,3]]"); // If you need to test interactivity use the `app` - await app.goto("/"); - await app.clickLink("/burgers"); - await page.waitForSelector("text=cheeseburger"); + await app.goto("/map", true); + + let html = await app.getHtml(); + expect(html).toMatch("[[1,1],[2,2],[3,3]]"); // If you're not sure what's going on, you can "poke" the app, it'll // automatically open up in your browser for 20 seconds, so be quick! @@ -121,6 +117,19 @@ test("[description of what you expect it to do]", async ({ page }) => { // Go check out the other tests to see what else you can do. }); +test("Maintains correct order of Set objects when hydrating", async ({ + page, +}) => { + let app = new PlaywrightFixture(appFixture, page); + let response = await fixture.requestDocument("/set"); + expect(await response.text()).toMatch("[1,2,3]"); + + await app.goto("/set", true); + + let html = await app.getHtml(); + expect(html).toMatch("[1,2,3]"); +}); + //////////////////////////////////////////////////////////////////////////////// // 💿 Finally, push your changes to your fork of React Router // and open a pull request! From fce0a7006e0111553c79cebe0b140fa6585b1ec4 Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Sat, 22 Feb 2025 16:45:44 -0700 Subject: [PATCH 2/3] Add bjohn465 to contributors.yml This is me signing the Contributor License Agreement (CLA). --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index cfbb9400d8..5ad6440c45 100644 --- a/contributors.yml +++ b/contributors.yml @@ -47,6 +47,7 @@ - BDomzalski - bhbs - bilalk711 +- bjohn465 - bobziroll - bravo-kernel - Brendonovich From 06e891296763fbfe71e286771da2409ac8537c7f Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Sun, 23 Feb 2025 12:15:46 -0700 Subject: [PATCH 3/3] Make matched text more specific So we are less likely to accidentally match text content that appears elsewhere in the page markup. --- integration/bug-report-test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration/bug-report-test.ts b/integration/bug-report-test.ts index 1a28328ae4..21164ff868 100644 --- a/integration/bug-report-test.ts +++ b/integration/bug-report-test.ts @@ -102,13 +102,13 @@ test("Maintains correct order of Map objects when hydrating", async ({ let app = new PlaywrightFixture(appFixture, page); // You can test any request your app might get using `fixture`. let response = await fixture.requestDocument("/map"); - expect(await response.text()).toMatch("[[1,1],[2,2],[3,3]]"); + expect(await response.text()).toMatch("
[[1,1],[2,2],[3,3]]
"); // If you need to test interactivity use the `app` await app.goto("/map", true); let html = await app.getHtml(); - expect(html).toMatch("[[1,1],[2,2],[3,3]]"); + expect(html).toMatch("
[[1,1],[2,2],[3,3]]
"); // If you're not sure what's going on, you can "poke" the app, it'll // automatically open up in your browser for 20 seconds, so be quick! @@ -122,12 +122,12 @@ test("Maintains correct order of Set objects when hydrating", async ({ }) => { let app = new PlaywrightFixture(appFixture, page); let response = await fixture.requestDocument("/set"); - expect(await response.text()).toMatch("[1,2,3]"); + expect(await response.text()).toMatch("
[1,2,3]
"); await app.goto("/set", true); let html = await app.getHtml(); - expect(html).toMatch("[1,2,3]"); + expect(html).toMatch("
[1,2,3]
"); }); ////////////////////////////////////////////////////////////////////////////////