diff --git a/.changeset/funny-hotels-repeat.md b/.changeset/funny-hotels-repeat.md new file mode 100644 index 0000000000..c44688ec77 --- /dev/null +++ b/.changeset/funny-hotels-repeat.md @@ -0,0 +1,6 @@ +--- +"react-router-dom": patch +"@remix-run/router": patch +--- + +Fix hrefs generated for createHashRouter diff --git a/contributors.yml b/contributors.yml index 82ff65bb5b..157443a895 100644 --- a/contributors.yml +++ b/contributors.yml @@ -22,6 +22,7 @@ - chrisngobanh - christopherchudzicki - christowiz +- codeape2 - coryhouse - cvbuelow - dauletbaev diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index e421e8599c..aeffc33c19 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -433,7 +433,7 @@ function testDomRouter( it("handles link navigations when using a basename", async () => { let testWindow = getWindow("/base/name/foo"); - let { container } = render( + render( Link to Foo Link to Bar - +
+ +
); } assertLocation(testWindow, "/base/name/foo"); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Foo - - - Link to Bar - -

- Foo Heading -

-
-
" - `); - expect(screen.getByText("Foo Heading")).toBeDefined(); + fireEvent.click(screen.getByText("Link to Bar")); await waitFor(() => screen.getByText("Bar Heading")); assertLocation(testWindow, "/base/name/bar"); @@ -508,8 +491,10 @@ function testDomRouter( return (
Link to Bar -

{navigation.state}

- +
+

{navigation.state}

+ +
); } @@ -522,62 +507,50 @@ function testDomRouter( return

{data?.message}

; } - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Bar - + expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "

idle

Foo

-
-
" - `); +
" + `); fireEvent.click(screen.getByText("Link to Bar")); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Bar - + expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "

loading

Foo

-
-
" - `); +
" + `); barDefer.resolve({ message: "Bar Loader" }); await waitFor(() => screen.getByText("idle")); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Bar - + expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "

idle

Bar Loader

-
-
" - `); +
" + `); }); it("handles link navigations with preventScrollReset", async () => { @@ -3664,8 +3637,10 @@ function testDomRouter(
Link to Foo Link to Bar -

{navigation.state}

- +
+

{navigation.state}

+ +
); } @@ -3687,83 +3662,56 @@ function testDomRouter( return

Bar Error:{error.message}

; } - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Foo - - - Link to Bar - -

- idle -

-

- Foo: - hydrated from foo -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Foo: + hydrated from foo +

" `); fireEvent.click(screen.getByText("Link to Bar")); barDefer.reject(new Error("Kaboom!")); await waitFor(() => screen.getByText("idle")); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Foo - - - Link to Bar - -

- idle -

-

- Bar Error: - Kaboom! -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Bar Error: + Kaboom! +

" `); fireEvent.click(screen.getByText("Link to Foo")); fooDefer.reject(new Error("Kaboom!")); await waitFor(() => screen.getByText("idle")); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Foo - - - Link to Bar - -

- idle -

-

- Foo Error: - Kaboom! -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Foo Error: + Kaboom! +

" - `); + `); }); it("renders navigation errors on parent elements", async () => { @@ -3803,8 +3751,10 @@ function testDomRouter(
Link to Foo Link to Bar -

{navigation.state}

- +
+

{navigation.state}

+ +
); } @@ -3825,27 +3775,18 @@ function testDomRouter( return

Bar:{data?.message}

; } - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Foo - - - Link to Bar - -

- idle -

-

- Foo: - hydrated from foo -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Foo: + hydrated from foo +

" `); @@ -3893,8 +3834,10 @@ function testDomRouter( return (
Link to Bar -

{navigation.state}

- +
+

{navigation.state}

+ +
); } @@ -3908,43 +3851,35 @@ function testDomRouter( return

Bar Error:{error.message}

; } - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Bar - -

- idle -

-

- Foo -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Foo +

" `); fireEvent.click(screen.getByText("Link to Bar")); barDefer.reject(new Error("Kaboom!")); await waitFor(() => screen.getByText("idle")); - expect(getHtml(container)).toMatchInlineSnapshot(` - "
-
- - Link to Bar - -

- idle -

-

- Bar Error: - Kaboom! -

-
+ expect(getHtml(container.querySelector("#output"))) + .toMatchInlineSnapshot(` + "
+

+ idle +

+

+ Bar Error: + Kaboom! +

" `); }); diff --git a/packages/react-router-dom/__tests__/link-href-test.tsx b/packages/react-router-dom/__tests__/link-href-test.tsx index 9ec368fc70..36a5d9ed6d 100644 --- a/packages/react-router-dom/__tests__/link-href-test.tsx +++ b/packages/react-router-dom/__tests__/link-href-test.tsx @@ -1,6 +1,17 @@ import * as React from "react"; +import { + BrowserRouter, + HashRouter, + Link, + MemoryRouter, + Outlet, + Route, + RouterProvider, + Routes, + createBrowserRouter, + createHashRouter, +} from "react-router-dom"; import * as TestRenderer from "react-test-renderer"; -import { MemoryRouter, Routes, Route, Link, Outlet } from "react-router-dom"; describe(" href", () => { describe("in a static route", () => { @@ -679,4 +690,72 @@ describe(" href", () => { ); }); }); + + describe("when using a browser router", () => { + it("renders proper for BrowserRouter", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + } /> + + + ); + }); + expect(renderer.root.findByType("a").props.href).toEqual( + "/path?search=value#hash" + ); + }); + + it("renders proper for createBrowserRouter", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + let router = createBrowserRouter([ + { + path: "/", + element: Link, + }, + ]); + renderer = TestRenderer.create(); + }); + expect(renderer.root.findByType("a").props.href).toEqual( + "/path?search=value#hash" + ); + }); + }); + + describe("when using a hash router", () => { + it("renders proper for HashRouter", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + } /> + + + ); + }); + expect(renderer.root.findByType("a").props.href).toEqual( + "#/path?search=value#hash" + ); + }); + + it("renders proper for createHashRouter", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + let router = createHashRouter([ + { + path: "/", + element: Link, + }, + ]); + renderer = TestRenderer.create(); + }); + expect(renderer.root.findByType("a").props.href).toEqual( + "#/path?search=value#hash" + ); + }); + }); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 2e7e495c38..c9f20fbe06 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1739,7 +1739,9 @@ export function createRouter(init: RouterInit): Router { navigate, fetch, revalidate, - createHref, + // Passthrough to history-aware createHref used by useHref so we get proper + // hash-aware URLs in DOM paths + createHref: (to: To) => init.history.createHref(to), getFetcher, deleteFetcher, dispose, @@ -1878,7 +1880,7 @@ export function unstable_createStaticHandler( ): Promise | Response> { let result: DataResult; if (!actionMatch.route.action) { - let href = createHref(new URL(request.url)); + let href = createServerHref(new URL(request.url)); result = getMethodNotAllowedResult(href); } else { result = await callLoaderOrAction( @@ -2136,7 +2138,7 @@ function normalizeNavigateOptions( path, submission: { formMethod: opts.formMethod, - formAction: createHref(parsePath(path)), + formAction: createServerHref(parsePath(path)), formEncType: (opts && opts.formEncType) || "application/x-www-form-urlencoded", formData: opts.formData, @@ -2696,7 +2698,7 @@ function getNotFoundMatches(routes: AgnosticDataRouteObject[]): { } function getMethodNotAllowedResult(path: Location | string): ErrorResult { - let href = typeof path === "string" ? path : createHref(path); + let href = typeof path === "string" ? path : createServerHref(path); console.warn( "You're trying to submit to a route that does not have an action. To " + "fix this, please add an `action` function to the route for " + @@ -2723,7 +2725,7 @@ function findRedirect(results: DataResult[]): RedirectResult | undefined { } // Create an href to represent a "server" URL without the hash -function createHref(location: Partial | Location | URL) { +function createServerHref(location: Partial | Location | URL) { return (location.pathname || "") + (location.search || ""); } @@ -2848,7 +2850,8 @@ function createURL(location: Location | string): URL { typeof window !== "undefined" && typeof window.location !== "undefined" ? window.location.origin : "unknown://unknown"; - let href = typeof location === "string" ? location : createHref(location); + let href = + typeof location === "string" ? location : createServerHref(location); return new URL(href, base); } //#endregion