From 6d9504ba92ee90611d35fc3ceaf2179da01b9769 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 18 Feb 2023 18:00:14 +0100 Subject: [PATCH 1/6] Hotfix: absolute URLs on server router --- packages/react-router-dom/index.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 882159e6cc..dca4f8fc1a 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -400,6 +400,8 @@ const isBrowser = typeof window.document !== "undefined" && typeof window.document.createElement !== "undefined"; +const ABSOLUTE_URL_REGEX = /^(?:[a-z][a-z0-9+.-]*:|\/\/)/i; + /** * The public API for rendering a history-aware . */ @@ -422,12 +424,14 @@ export const Link = React.forwardRef( let absoluteHref; let isExternal = false; - if ( - isBrowser && - typeof to === "string" && - /^(?:[a-z][a-z0-9+.-]*:|\/\/)/i.test(to) - ) { + const isAbsoluteUrl = typeof to === "string" && ABSOLUTE_URL_REGEX.test(to); + + // set the absoluteHref if the to is an absolute url + if (isAbsoluteUrl) { absoluteHref = to; + } + + if (isBrowser && isAbsoluteUrl) { let currentUrl = new URL(window.location.href); let targetUrl = to.startsWith("//") ? new URL(currentUrl.protocol + to) From 930d054af0e92f7ba7680d55535a8bf18cda87a5 Mon Sep 17 00:00:00 2001 From: Thomas Verleye Date: Sat, 18 Feb 2023 18:05:32 +0100 Subject: [PATCH 2/6] Update contributors.yml --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index 662edea87a..0d2a826d26 100644 --- a/contributors.yml +++ b/contributors.yml @@ -162,6 +162,7 @@ - tanayv - theostavrides - thisiskartik +- thomasverleye - ThornWu - timdorr - TkDodo From 6bc783a6cf89994fa92b1e6de6a8d417ea8376a8 Mon Sep 17 00:00:00 2001 From: Thomas Date: Sat, 18 Feb 2023 18:48:44 +0100 Subject: [PATCH 3/6] Create test for Link in StaticRouter Context --- .../link-href-static-router-test.tsx | 80 +++++++++++++++++++ .../polyfills/SubmitEvent.submitter.ts | 40 +++++----- 2 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 packages/react-router-dom/__tests__/link-href-static-router-test.tsx diff --git a/packages/react-router-dom/__tests__/link-href-static-router-test.tsx b/packages/react-router-dom/__tests__/link-href-static-router-test.tsx new file mode 100644 index 0000000000..3b299870b2 --- /dev/null +++ b/packages/react-router-dom/__tests__/link-href-static-router-test.tsx @@ -0,0 +1,80 @@ +/** + * @jest-environment node + */ +import * as React from "react"; +import * as ReactDOMServer from "react-dom/server"; +import type { StaticHandlerContext } from "@remix-run/router"; +import { createStaticHandler } from "@remix-run/router"; +import { Link, Outlet } from "react-router-dom"; +import { + createStaticRouter, + StaticRouterProvider, +} from "react-router-dom/server"; + +beforeEach(() => { + jest.spyOn(console, "warn").mockImplementation(() => {}); +}); + +describe("A ", () => { + it("renders an initialized router with all possible links", async () => { + function HooksChecker1() { + return <> + + the path + the absolute url + the absolute url with a mailto: + ; + } + + function HooksChecker2() { + return ( + <> +

👋

+ Other + + ); + } + + let routes = [ + { + path: "the", + loader: () => ({ + key1: "value1", + }), + element: , + handle: "1", + children: [ + { + path: "path", + loader: () => ({ + key2: "value2", + }), + element: , + handle: "2", + }, + ], + }, + ]; + let { query } = createStaticHandler(routes); + + let context = (await query( + new Request("http://localhost/the/path", { + signal: new AbortController().signal, + }) + )) as StaticHandlerContext; + + let html = ReactDOMServer.renderToStaticMarkup( + + + + ); + expect(html).toMatch("

👋

"); + expect(html).toMatch(''); + expect(html).toMatch(''); + expect(html).toMatch(''); + expect(html).toMatch(''); + }); +}); diff --git a/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts b/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts index 84f73710ae..f5095fdd49 100644 --- a/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts +++ b/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts @@ -6,23 +6,25 @@ if ( const setImmediate = (fn, ...args) => global.setTimeout(fn, 0, ...args); let maybeSubmitter; - window.addEventListener( - "click", - (event) => { - if ((event.target as any)?.form) maybeSubmitter = event.target; - setImmediate(() => { - // if this click doesn't imminently trigger a submit event, then forget it - maybeSubmitter = undefined; - }); - }, - { capture: true } - ); - window.addEventListener( - "submit", - (event: any) => { - if (maybeSubmitter?.form === event.target) - event.submitter = maybeSubmitter; - }, - { capture: true } - ); + if (typeof window !== "undefined") { + window.addEventListener( + "click", + (event) => { + if ((event.target as any)?.form) maybeSubmitter = event.target; + setImmediate(() => { + // if this click doesn't imminently trigger a submit event, then forget it + maybeSubmitter = undefined; + }); + }, + { capture: true } + ); + window.addEventListener( + "submit", + (event: any) => { + if (maybeSubmitter?.form === event.target) + event.submitter = maybeSubmitter; + }, + { capture: true } + ); + } } From 0a2ca8d001e88b19f5beac19e51adc7d8fa8be0a Mon Sep 17 00:00:00 2001 From: Thomas Date: Tue, 21 Feb 2023 20:30:55 +0100 Subject: [PATCH 4/6] =?UTF-8?q?Link=20Component=20=E2=80=94=20Reassure=20t?= =?UTF-8?q?he=20type=20of=20`to`=20is=20string=20when=20checking=20origin?= =?UTF-8?q?=20of=20absolute=20url?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/react-router-dom/index.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index dca4f8fc1a..e19be72a0c 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -424,14 +424,15 @@ export const Link = React.forwardRef( let absoluteHref; let isExternal = false; - const isAbsoluteUrl = typeof to === "string" && ABSOLUTE_URL_REGEX.test(to); - // set the absoluteHref if the to is an absolute url - if (isAbsoluteUrl) { + // making sure StaticRouter renders correct href + if (typeof to === "string" && ABSOLUTE_URL_REGEX.test(to)) { absoluteHref = to; } - if (isBrowser && isAbsoluteUrl) { + // If in browser, we can check if to url can be stripped + // only when the origin is the same as the current url + if (isBrowser && typeof to === "string" && ABSOLUTE_URL_REGEX.test(to)) { let currentUrl = new URL(window.location.href); let targetUrl = to.startsWith("//") ? new URL(currentUrl.protocol + to) From 365ac86b03205b8f0c44dc5b978914eec81db8f9 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 22 Feb 2023 09:33:01 -0500 Subject: [PATCH 5/6] Combine tests and streamline implementation --- .../__tests__/data-static-router-test.tsx | 42 ++++++++++ .../link-href-static-router-test.tsx | 80 ------------------- .../polyfills/SubmitEvent.submitter.ts | 45 +++++------ packages/react-router-dom/__tests__/setup.ts | 3 +- packages/react-router-dom/index.tsx | 30 ++++--- 5 files changed, 80 insertions(+), 120 deletions(-) delete mode 100644 packages/react-router-dom/__tests__/link-href-static-router-test.tsx diff --git a/packages/react-router-dom/__tests__/data-static-router-test.tsx b/packages/react-router-dom/__tests__/data-static-router-test.tsx index 3f280d287a..1562a022f8 100644 --- a/packages/react-router-dom/__tests__/data-static-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-static-router-test.tsx @@ -1,3 +1,7 @@ +/** + * @jest-environment node + */ + import * as React from "react"; import * as ReactDOMServer from "react-dom/server"; import type { StaticHandlerContext } from "@remix-run/router"; @@ -611,6 +615,44 @@ describe("A ", () => { `); }); + it("renders absolute links correctly", async () => { + let routes = [ + { + path: "/", + element: ( + <> + relative path + absolute same-origin url + absolute different-origin url + absolute mailto: url + + ), + }, + ]; + let { query } = createStaticHandler(routes); + + let context = (await query( + new Request("http://localhost/", { + signal: new AbortController().signal, + }) + )) as StaticHandlerContext; + + let html = ReactDOMServer.renderToStaticMarkup( + + + + ); + expect(html).toMatch( + 'relative path' + + 'absolute same-origin url' + + 'absolute different-origin url' + + 'absolute mailto: url' + ); + }); + describe("boundary tracking", () => { it("tracks the deepest boundary during render", async () => { let routes = [ diff --git a/packages/react-router-dom/__tests__/link-href-static-router-test.tsx b/packages/react-router-dom/__tests__/link-href-static-router-test.tsx deleted file mode 100644 index 3b299870b2..0000000000 --- a/packages/react-router-dom/__tests__/link-href-static-router-test.tsx +++ /dev/null @@ -1,80 +0,0 @@ -/** - * @jest-environment node - */ -import * as React from "react"; -import * as ReactDOMServer from "react-dom/server"; -import type { StaticHandlerContext } from "@remix-run/router"; -import { createStaticHandler } from "@remix-run/router"; -import { Link, Outlet } from "react-router-dom"; -import { - createStaticRouter, - StaticRouterProvider, -} from "react-router-dom/server"; - -beforeEach(() => { - jest.spyOn(console, "warn").mockImplementation(() => {}); -}); - -describe("A ", () => { - it("renders an initialized router with all possible links", async () => { - function HooksChecker1() { - return <> - - the path - the absolute url - the absolute url with a mailto: - ; - } - - function HooksChecker2() { - return ( - <> -

👋

- Other - - ); - } - - let routes = [ - { - path: "the", - loader: () => ({ - key1: "value1", - }), - element: , - handle: "1", - children: [ - { - path: "path", - loader: () => ({ - key2: "value2", - }), - element: , - handle: "2", - }, - ], - }, - ]; - let { query } = createStaticHandler(routes); - - let context = (await query( - new Request("http://localhost/the/path", { - signal: new AbortController().signal, - }) - )) as StaticHandlerContext; - - let html = ReactDOMServer.renderToStaticMarkup( - - - - ); - expect(html).toMatch("

👋

"); - expect(html).toMatch(''); - expect(html).toMatch(''); - expect(html).toMatch(''); - expect(html).toMatch(''); - }); -}); diff --git a/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts b/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts index f5095fdd49..323c8964f2 100644 --- a/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts +++ b/packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts @@ -1,30 +1,29 @@ // Polyfill jsdom SubmitEvent.submitter, until https://github.com/jsdom/jsdom/pull/3481 is merged if ( - typeof SubmitEvent === "undefined" || - !SubmitEvent.prototype.hasOwnProperty("submitter") + typeof window !== "undefined" && + (typeof SubmitEvent === "undefined" || + !SubmitEvent.prototype.hasOwnProperty("submitter")) ) { const setImmediate = (fn, ...args) => global.setTimeout(fn, 0, ...args); let maybeSubmitter; - if (typeof window !== "undefined") { - window.addEventListener( - "click", - (event) => { - if ((event.target as any)?.form) maybeSubmitter = event.target; - setImmediate(() => { - // if this click doesn't imminently trigger a submit event, then forget it - maybeSubmitter = undefined; - }); - }, - { capture: true } - ); - window.addEventListener( - "submit", - (event: any) => { - if (maybeSubmitter?.form === event.target) - event.submitter = maybeSubmitter; - }, - { capture: true } - ); - } + window.addEventListener( + "click", + (event) => { + if ((event.target as any)?.form) maybeSubmitter = event.target; + setImmediate(() => { + // if this click doesn't imminently trigger a submit event, then forget it + maybeSubmitter = undefined; + }); + }, + { capture: true } + ); + window.addEventListener( + "submit", + (event: any) => { + if (maybeSubmitter?.form === event.target) + event.submitter = maybeSubmitter; + }, + { capture: true } + ); } diff --git a/packages/react-router-dom/__tests__/setup.ts b/packages/react-router-dom/__tests__/setup.ts index 846e483ef1..576b8289af 100644 --- a/packages/react-router-dom/__tests__/setup.ts +++ b/packages/react-router-dom/__tests__/setup.ts @@ -2,7 +2,7 @@ import { TextEncoder as NodeTextEncoder, TextDecoder as NodeTextDecoder, } from "util"; -import { fetch, Request, Response } from "@remix-run/web-fetch"; +import { fetch, Request, Response, Headers } from "@remix-run/web-fetch"; import { AbortController as NodeAbortController } from "abort-controller"; import "./polyfills/SubmitEvent.submitter"; @@ -22,6 +22,7 @@ if (!globalThis.fetch) { // web-std/fetch Response does not currently implement Response.error() // @ts-expect-error globalThis.Response = Response; + globalThis.Headers = Headers; } if (!globalThis.AbortController) { diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index e19be72a0c..fc6dfd4a4c 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -424,24 +424,22 @@ export const Link = React.forwardRef( let absoluteHref; let isExternal = false; - // set the absoluteHref if the to is an absolute url - // making sure StaticRouter renders correct href if (typeof to === "string" && ABSOLUTE_URL_REGEX.test(to)) { + // Render the absolute href server- and client-side absoluteHref = to; - } - - // If in browser, we can check if to url can be stripped - // only when the origin is the same as the current url - if (isBrowser && typeof to === "string" && ABSOLUTE_URL_REGEX.test(to)) { - let currentUrl = new URL(window.location.href); - let targetUrl = to.startsWith("//") - ? new URL(currentUrl.protocol + to) - : new URL(to); - if (targetUrl.origin === currentUrl.origin) { - // Strip the protocol/origin for same-origin absolute URLs - to = targetUrl.pathname + targetUrl.search + targetUrl.hash; - } else { - isExternal = true; + + // Only check for external origins client-side + if (isBrowser) { + let currentUrl = new URL(window.location.href); + let targetUrl = to.startsWith("//") + ? new URL(currentUrl.protocol + to) + : new URL(to); + if (targetUrl.origin === currentUrl.origin) { + // Strip the protocol/origin for same-origin absolute URLs + to = targetUrl.pathname + targetUrl.search + targetUrl.hash; + } else { + isExternal = true; + } } } From 7244657e52e839ba720370410fd0705cd9452f98 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 22 Feb 2023 09:35:32 -0500 Subject: [PATCH 6/6] Add changeset --- .changeset/lovely-cheetahs-sip.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/lovely-cheetahs-sip.md diff --git a/.changeset/lovely-cheetahs-sip.md b/.changeset/lovely-cheetahs-sip.md new file mode 100644 index 0000000000..28500c29ed --- /dev/null +++ b/.changeset/lovely-cheetahs-sip.md @@ -0,0 +1,5 @@ +--- +"react-router-dom": patch +--- + +Fix SSR of absolute Link urls