diff --git a/.changeset/great-owls-juggle.md b/.changeset/great-owls-juggle.md new file mode 100644 index 0000000000..478b0c48f8 --- /dev/null +++ b/.changeset/great-owls-juggle.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": patch +--- + +Fix URL creation with memory histories diff --git a/package.json b/package.json index 53ba67abbc..e1fc38d9d0 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "@types/semver": "^7.3.8", "@typescript-eslint/eslint-plugin": "^4.28.3", "@typescript-eslint/parser": "^4.28.3", + "abort-controller": "^3.0.0", "babel-eslint": "^10.1.0", "babel-jest": "^26.6.3", "babel-plugin-dev-expression": "^0.2.2", 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 b1c8ebace3..4b79ef7156 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -4263,7 +4263,7 @@ function createDeferred() { function getWindowImpl(initialUrl: string, isHash = false): Window { // Need to use our own custom DOM in order to get a working history - const dom = new JSDOM(``, { url: "https://remix.run/" }); + const dom = new JSDOM(``, { url: "http://localhost/" }); dom.window.history.replaceState(null, "", (isHash ? "#" : "") + initialUrl); return dom.window as unknown as Window; } diff --git a/packages/router/__tests__/browser-test.ts b/packages/router/__tests__/browser-test.ts index a5fb3c9e9a..b699f501ae 100644 --- a/packages/router/__tests__/browser-test.ts +++ b/packages/router/__tests__/browser-test.ts @@ -1,7 +1,3 @@ -/** - * @jest-environment ./__tests__/custom-environment.js - */ - /* eslint-disable jest/expect-expect */ import { JSDOM } from "jsdom"; diff --git a/packages/router/__tests__/custom-environment.js b/packages/router/__tests__/custom-environment.js deleted file mode 100644 index b3ff337de5..0000000000 --- a/packages/router/__tests__/custom-environment.js +++ /dev/null @@ -1,19 +0,0 @@ -const Environment = require("jest-environment-jsdom"); - -/** - * A custom environment to set the TextEncoder that is required by JSDOM - * See: https://stackoverflow.com/questions/57712235/referenceerror-textencoder-is-not-defined-when-running-react-scripts-test - */ -module.exports = class CustomTestEnvironment extends Environment { - async setup() { - await super.setup(); - if (typeof this.global.TextEncoder === "undefined") { - const { TextEncoder } = require("util"); - this.global.TextEncoder = TextEncoder; - } - if (typeof this.global.TextDecoder === "undefined") { - const { TextDecoder } = require("util"); - this.global.TextDecoder = TextDecoder; - } - } -}; diff --git a/packages/router/__tests__/hash-base-test.ts b/packages/router/__tests__/hash-base-test.ts index 07abd0fd66..511df27394 100644 --- a/packages/router/__tests__/hash-base-test.ts +++ b/packages/router/__tests__/hash-base-test.ts @@ -1,7 +1,3 @@ -/** - * @jest-environment ./__tests__/custom-environment.js - */ - import { JSDOM } from "jsdom"; import type { HashHistory } from "@remix-run/router"; diff --git a/packages/router/__tests__/hash-test.ts b/packages/router/__tests__/hash-test.ts index 0fec44766f..9a2f166f82 100644 --- a/packages/router/__tests__/hash-test.ts +++ b/packages/router/__tests__/hash-test.ts @@ -1,7 +1,3 @@ -/** - * @jest-environment ./__tests__/custom-environment.js - */ - /* eslint-disable jest/expect-expect */ import { JSDOM } from "jsdom"; diff --git a/packages/router/__tests__/router-memory-test.ts b/packages/router/__tests__/router-memory-test.ts new file mode 100644 index 0000000000..2a1635fac4 --- /dev/null +++ b/packages/router/__tests__/router-memory-test.ts @@ -0,0 +1,107 @@ +/** + * @jest-environment node + */ + +import { createMemoryHistory, createRouter } from "../index"; + +// This suite of tests specifically runs in the node jest environment to catch +// issues when window is not present +describe("a memory router", () => { + it("initializes properly", async () => { + let router = createRouter({ + routes: [ + { + path: "/", + }, + ], + history: createMemoryHistory(), + }); + expect(router.state).toEqual({ + historyAction: "POP", + loaderData: {}, + actionData: null, + errors: null, + location: { + hash: "", + key: expect.any(String), + pathname: "/", + search: "", + state: null, + }, + matches: [ + { + params: {}, + pathname: "/", + pathnameBase: "/", + route: { + id: "0", + path: "/", + }, + }, + ], + initialized: true, + navigation: { + location: undefined, + state: "idle", + }, + preventScrollReset: false, + restoreScrollPosition: null, + revalidation: "idle", + fetchers: new Map(), + }); + router.dispose(); + }); + + it("can create Requests without window", async () => { + let loaderSpy = jest.fn(); + let router = createRouter({ + routes: [ + { + path: "/", + }, + { + path: "/a", + loader: loaderSpy, + }, + ], + history: createMemoryHistory(), + }); + + router.navigate("/a"); + expect(loaderSpy.mock.calls[0][0].request.url).toBe("http://localhost/a"); + router.dispose(); + }); + + it("can create URLs without window", async () => { + let shouldRevalidateSpy = jest.fn(); + + let router = createRouter({ + routes: [ + { + path: "/", + loader: () => "ROOT", + shouldRevalidate: shouldRevalidateSpy, + children: [ + { + index: true, + }, + { + path: "a", + }, + ], + }, + ], + history: createMemoryHistory(), + hydrationData: { loaderData: { "0": "ROOT" } }, + }); + + router.navigate("/a"); + expect(shouldRevalidateSpy.mock.calls[0][0].currentUrl.toString()).toBe( + "http://localhost/" + ); + expect(shouldRevalidateSpy.mock.calls[0][0].nextUrl.toString()).toBe( + "http://localhost/a" + ); + router.dispose(); + }); +}); diff --git a/packages/router/__tests__/setup.ts b/packages/router/__tests__/setup.ts index 6d9d8d4a16..2588f2abd7 100644 --- a/packages/router/__tests__/setup.ts +++ b/packages/router/__tests__/setup.ts @@ -1,4 +1,9 @@ +import { + TextEncoder as NodeTextEncoder, + TextDecoder as NodeTextDecoder, +} from "util"; import { fetch, Request, Response } from "@remix-run/web-fetch"; +import { AbortController as NodeAbortController } from "abort-controller"; if (!globalThis.fetch) { // Built-in lib.dom.d.ts expects `fetch(Request | string, ...)` but the web @@ -13,3 +18,14 @@ if (!globalThis.fetch) { // @ts-expect-error globalThis.Response = Response; } + +if (!globalThis.AbortController) { + // @ts-expect-error + globalThis.AbortController = NodeAbortController; +} + +if (!globalThis.TextEncoder || !globalThis.TextDecoder) { + globalThis.TextEncoder = NodeTextEncoder; + // @ts-expect-error + globalThis.TextDecoder = NodeTextDecoder; +} diff --git a/packages/router/history.ts b/packages/router/history.ts index e5117817b7..e8b6c10f7d 100644 --- a/packages/router/history.ts +++ b/packages/router/history.ts @@ -125,6 +125,13 @@ export interface History { */ createHref(to: To): string; + /** + * Returns a URL for the given `to` value + * + * @param to - The destination URL + */ + createURL(to: To): URL; + /** * Encode a location the same way window.history would do (no-op for memory * history) so we ensure our PUSH/REPLACE navigations for data routers @@ -255,6 +262,10 @@ export function createMemoryHistory( return location; } + function createHref(to: To) { + return typeof to === "string" ? to : createPath(to); + } + let history: MemoryHistory = { get index() { return index; @@ -265,8 +276,9 @@ export function createMemoryHistory( get location() { return getCurrentLocation(); }, - createHref(to) { - return typeof to === "string" ? to : createPath(to); + createHref, + createURL(to) { + return new URL(createHref(to), "http://localhost"); }, encodeLocation(to: To) { let path = typeof to === "string" ? parsePath(to) : to; @@ -558,24 +570,6 @@ export function parsePath(path: string): Partial { return parsedPath; } -export function createClientSideURL(location: Location | string): URL { - // window.location.origin is "null" (the literal string value) in Firefox - // under certain conditions, notably when serving from a local HTML file - // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297 - let base = - typeof window !== "undefined" && - typeof window.location !== "undefined" && - window.location.origin !== "null" - ? window.location.origin - : window.location.href; - let href = typeof location === "string" ? location : createPath(location); - invariant( - base, - `No window.location.(origin|href) available to create URL for href: ${href}` - ); - return new URL(href, base); -} - export interface UrlHistory extends History {} export type UrlHistoryOptions = { @@ -637,6 +631,23 @@ function getUrlBasedHistory( } } + function createURL(to: To): URL { + // window.location.origin is "null" (the literal string value) in Firefox + // under certain conditions, notably when serving from a local HTML file + // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297 + let base = + window.location.origin !== "null" + ? window.location.origin + : window.location.href; + + let href = typeof to === "string" ? to : createPath(to); + invariant( + base, + `No window.location.(origin|href) available to create URL for href: ${href}` + ); + return new URL(href, base); + } + let history: History = { get action() { return action; @@ -659,11 +670,10 @@ function getUrlBasedHistory( createHref(to) { return createHref(window, to); }, + createURL, encodeLocation(to) { // Encode a Location the same way window.location would - let url = createClientSideURL( - typeof to === "string" ? to : createPath(to) - ); + let url = createURL(to); return { pathname: url.pathname, search: url.search, diff --git a/packages/router/router.ts b/packages/router/router.ts index 6365a49e58..50426da2aa 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -3,7 +3,6 @@ import { Action as HistoryAction, createLocation, createPath, - createClientSideURL, invariant, parsePath, } from "./history"; @@ -957,6 +956,7 @@ export function createRouter(init: RouterInit): Router { // Create a controller/Request for this navigation pendingNavigationController = new AbortController(); let request = createClientSideRequest( + init.history, location, pendingNavigationController.signal, opts && opts.submission @@ -1167,6 +1167,7 @@ export function createRouter(init: RouterInit): Router { : undefined; let [matchesToLoad, revalidatingFetchers] = getMatchesToLoad( + init.history, state, matches, activeSubmission, @@ -1380,6 +1381,7 @@ export function createRouter(init: RouterInit): Router { // Call the action for the fetcher let abortController = new AbortController(); let fetchRequest = createClientSideRequest( + init.history, path, abortController.signal, submission @@ -1434,6 +1436,8 @@ export function createRouter(init: RouterInit): Router { // in the middle of a navigation let nextLocation = state.navigation.location || state.location; let revalidationRequest = createClientSideRequest( + init.history, + nextLocation, abortController.signal ); @@ -1456,6 +1460,7 @@ export function createRouter(init: RouterInit): Router { state.fetchers.set(key, loadFetcher); let [matchesToLoad, revalidatingFetchers] = getMatchesToLoad( + init.history, state, matches, submission, @@ -1599,7 +1604,11 @@ export function createRouter(init: RouterInit): Router { // Call the loader for this fetcher route match let abortController = new AbortController(); - let fetchRequest = createClientSideRequest(path, abortController.signal); + let fetchRequest = createClientSideRequest( + init.history, + path, + abortController.signal + ); fetchControllers.set(key, abortController); let result: DataResult = await callLoaderOrAction( "loader", @@ -1719,7 +1728,7 @@ export function createRouter(init: RouterInit): Router { // Check if this an external redirect that goes to a new origin if (typeof window?.location !== "undefined") { - let newOrigin = createClientSideURL(redirect.location).origin; + let newOrigin = init.history.createURL(redirect.location).origin; if (window.location.origin !== newOrigin) { if (replace) { window.location.replace(redirect.location); @@ -1796,7 +1805,7 @@ export function createRouter(init: RouterInit): Router { ...fetchersToLoad.map(([, href, match, fetchMatches]) => callLoaderOrAction( "loader", - createClientSideRequest(href, request.signal), + createClientSideRequest(init.history, href, request.signal), match, fetchMatches, router.basename @@ -2595,6 +2604,7 @@ function getLoaderMatchesUntilBoundary( } function getMatchesToLoad( + history: History, state: RouterState, matches: AgnosticDataRouteMatch[], submission: Submission | undefined, @@ -2622,6 +2632,7 @@ function getMatchesToLoad( // If this route had a pending deferred cancelled it must be revalidated cancelledDeferredRoutes.some((id) => id === match.route.id) || shouldRevalidateLoader( + history, state.location, state.matches[index], submission, @@ -2641,6 +2652,7 @@ function getMatchesToLoad( revalidatingFetchers.push([key, href, match, fetchMatches]); } else if (isRevalidationRequired) { let shouldRevalidate = shouldRevalidateLoader( + history, href, match, submission, @@ -2694,6 +2706,7 @@ function isNewRouteInstance( } function shouldRevalidateLoader( + history: History, currentLocation: string | Location, currentMatch: AgnosticDataRouteMatch, submission: Submission | undefined, @@ -2702,9 +2715,9 @@ function shouldRevalidateLoader( isRevalidationRequired: boolean, actionResult: DataResult | undefined ) { - let currentUrl = createClientSideURL(currentLocation); + let currentUrl = history.createURL(currentLocation); let currentParams = currentMatch.params; - let nextUrl = createClientSideURL(location); + let nextUrl = history.createURL(location); let nextParams = match.params; // This is the default implementation as to when we revalidate. If the route @@ -2893,11 +2906,12 @@ async function callLoaderOrAction( // client-side navigations and fetches. During SSR we will always have a // Request instance from the static handler (query/queryRoute) function createClientSideRequest( + history: History, location: string | Location, signal: AbortSignal, submission?: Submission ): Request { - let url = createClientSideURL(stripHashFromPath(location)).toString(); + let url = history.createURL(stripHashFromPath(location)).toString(); let init: RequestInit = { signal }; if (submission && isMutationMethod(submission.formMethod)) {