From b2bc694a61d39bc383b0d9e25f0d164f50472033 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 29 Apr 2025 12:01:22 -0400 Subject: [PATCH 1/8] Don't bundle react-router in react-router/dom export --- .changeset/breezy-pets-hug.md | 5 +++++ packages/react-router/tsup.config.ts | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/breezy-pets-hug.md diff --git a/.changeset/breezy-pets-hug.md b/.changeset/breezy-pets-hug.md new file mode 100644 index 0000000000..2c07e29f33 --- /dev/null +++ b/.changeset/breezy-pets-hug.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Don't bundle `react-router` in `react-router/dom` CJS export diff --git a/packages/react-router/tsup.config.ts b/packages/react-router/tsup.config.ts index 5d93e3e81b..f047508d79 100644 --- a/packages/react-router/tsup.config.ts +++ b/packages/react-router/tsup.config.ts @@ -12,6 +12,7 @@ const config = (enableDevWarnings: boolean) => { clean: false, entry, + external: ["react", "react-dom", "react-router"], format: ["cjs"], outDir: enableDevWarnings ? "dist/development" : "dist/production", dts: true, From 84425ce70cfcf4c2dafba7a26d2a725b9917b421 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 30 Apr 2025 09:41:04 -0400 Subject: [PATCH 2/8] Add externals to ESM build and remove unneeded react externals --- packages/react-router/tsup.config.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react-router/tsup.config.ts b/packages/react-router/tsup.config.ts index f047508d79..26b038f205 100644 --- a/packages/react-router/tsup.config.ts +++ b/packages/react-router/tsup.config.ts @@ -12,8 +12,9 @@ const config = (enableDevWarnings: boolean) => { clean: false, entry, - external: ["react", "react-dom", "react-router"], format: ["cjs"], + // Don't bundle `react-router` in sub-exports (i.e., `react-router/dom`) + external: ["react-router"], outDir: enableDevWarnings ? "dist/development" : "dist/production", dts: true, banner: { @@ -29,6 +30,8 @@ const config = (enableDevWarnings: boolean) => clean: false, entry, format: ["esm"], + // Don't bundle `react-router` in sub-exports (i.e., `react-router/dom`) + external: ["react-router"], outDir: enableDevWarnings ? "dist/development" : "dist/production", dts: true, banner: { From fc893df145e692269e8d44f750e9aeecc4575073 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 30 Apr 2025 13:47:49 -0400 Subject: [PATCH 3/8] Fix e2e tests --- integration/prefetch-test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index bee1b75991..3e3583973e 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -166,7 +166,8 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - "#nav link[rel='modulepreload'][href^='/assets/chunk-']", + // Rollup bundles RR (and transitively React) into an index chunk + "#nav link[rel='modulepreload'][href^='/assets/index-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -219,10 +220,13 @@ test.describe("prefetch", () => { "#nav link[rel='modulepreload'][href^='/assets/with-props-']", { state: "attached" } ); + + console.log(await app.getHtml()); await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - "#nav link[rel='modulepreload'][href^='/assets/chunk-']", + // Rollup bundles RR (and transitively React) into an index chunk + "#nav link[rel='modulepreload'][href^='/assets/index-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -241,7 +245,8 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - "#nav link[rel='modulepreload'][href^='/assets/chunk-']", + // Rollup bundles RR (and transitively React) into an index chunk + "#nav link[rel='modulepreload'][href^='/assets/index-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -321,7 +326,8 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - "#nav link[rel='modulepreload'][href^='/assets/chunk-']", + // Rollup bundles RR (and transitively React) into an index chunk + "#nav link[rel='modulepreload'][href^='/assets/index-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -340,7 +346,8 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - "#nav link[rel='modulepreload'][href^='/assets/chunk-']", + // Rollup bundles RR (and transitively React) into an index chunk + "#nav link[rel='modulepreload'][href^='/assets/index-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } From 1fc71ae40891a44981fae52d79dc94ddb166d8eb Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 30 Apr 2025 13:48:12 -0400 Subject: [PATCH 4/8] Remove console --- integration/prefetch-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index 3e3583973e..405dee2c51 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -221,7 +221,6 @@ test.describe("prefetch", () => { { state: "attached" } ); - console.log(await app.getHtml()); await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ From 220239f0a1982fe5e6142ccfc7a28e36f6fa32dd Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 30 Apr 2025 13:48:28 -0400 Subject: [PATCH 5/8] Remove blank line --- integration/prefetch-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index 405dee2c51..236b071eef 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -220,7 +220,6 @@ test.describe("prefetch", () => { "#nav link[rel='modulepreload'][href^='/assets/with-props-']", { state: "attached" } ); - await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ From 87d545ca85911b729f2aadca49fd0e0a9ec28a44 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 1 May 2025 09:36:53 -0400 Subject: [PATCH 6/8] Don't include external in ESM config --- packages/react-router/tsup.config.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-router/tsup.config.ts b/packages/react-router/tsup.config.ts index 26b038f205..cd5f0def14 100644 --- a/packages/react-router/tsup.config.ts +++ b/packages/react-router/tsup.config.ts @@ -30,8 +30,10 @@ const config = (enableDevWarnings: boolean) => clean: false, entry, format: ["esm"], - // Don't bundle `react-router` in sub-exports (i.e., `react-router/dom`) - external: ["react-router"], + // We don't do the external thing for `react-router` here because it + // doesn't get bundled by default in the ESM build, and when we tried it + // in https://github.com/remix-run/react-router/pull/13497 it changed up + // some chunk creation that we didn't want to risk having any side effects outDir: enableDevWarnings ? "dist/development" : "dist/production", dts: true, banner: { From 488e330727c48bc4611e531d0cc1530ccd99ae51 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 1 May 2025 09:37:02 -0400 Subject: [PATCH 7/8] Revert "Fix e2e tests" This reverts commit fc893df145e692269e8d44f750e9aeecc4575073. --- integration/prefetch-test.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts index 236b071eef..bee1b75991 100644 --- a/integration/prefetch-test.ts +++ b/integration/prefetch-test.ts @@ -166,8 +166,7 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - // Rollup bundles RR (and transitively React) into an index chunk - "#nav link[rel='modulepreload'][href^='/assets/index-']", + "#nav link[rel='modulepreload'][href^='/assets/chunk-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -223,8 +222,7 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - // Rollup bundles RR (and transitively React) into an index chunk - "#nav link[rel='modulepreload'][href^='/assets/index-']", + "#nav link[rel='modulepreload'][href^='/assets/chunk-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -243,8 +241,7 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - // Rollup bundles RR (and transitively React) into an index chunk - "#nav link[rel='modulepreload'][href^='/assets/index-']", + "#nav link[rel='modulepreload'][href^='/assets/chunk-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -324,8 +321,7 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - // Rollup bundles RR (and transitively React) into an index chunk - "#nav link[rel='modulepreload'][href^='/assets/index-']", + "#nav link[rel='modulepreload'][href^='/assets/chunk-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } @@ -344,8 +340,7 @@ test.describe("prefetch", () => { await page.waitForSelector( // Look for either Rollup or Rolldown chunks [ - // Rollup bundles RR (and transitively React) into an index chunk - "#nav link[rel='modulepreload'][href^='/assets/index-']", + "#nav link[rel='modulepreload'][href^='/assets/chunk-']", "#nav link[rel='modulepreload'][href^='/assets/jsx-runtime-']", ].join(","), { state: "attached" } From 818f8e08d79e233d5d61af96b0e9b71e773a3023 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 1 May 2025 09:44:42 -0400 Subject: [PATCH 8/8] Add unit test --- .../__tests__/dom/dom-export-test.tsx | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 packages/react-router/__tests__/dom/dom-export-test.tsx diff --git a/packages/react-router/__tests__/dom/dom-export-test.tsx b/packages/react-router/__tests__/dom/dom-export-test.tsx new file mode 100644 index 0000000000..2ecd4172b0 --- /dev/null +++ b/packages/react-router/__tests__/dom/dom-export-test.tsx @@ -0,0 +1,40 @@ +import * as React from "react"; + +import { render, screen } from "@testing-library/react"; +import { createMemoryRouter, useParams } from "react-router"; +import { RouterProvider } from "react-router/dom"; + +describe("react-router/dom", () => { + function ShowParams() { + return 
{JSON.stringify(useParams())}
; + } + + describe("Does not bundle react-router causing duplicate context issues", () => { + it("with route provider shows the url params", async () => { + const router = createMemoryRouter( + [ + { + path: "/blog/:slug", + element: , + }, + ], + { + initialEntries: ["/blog/react-router"], + } + ); + + // When react-router was bundled in CJS scenarios, this `react-router/dom` + // version of `RouterProvider` caused duplicate contexts and we would not + // find the param values + render(); + + expect(await screen.findByTestId("params")).toMatchInlineSnapshot(` + 
+        {"slug":"react-router"}
+
+ `); + }); + }); +});