Fix URL creation with memory history (#9814)

* Fix URL creation with memory history * Create history-aware URLs * add changeset * Fix hash test
remix-run · Jan 11, 2023 · b154367 · b154367
1 parent ce2bb3f
commit b154367
Show file tree

Hide file tree

Showing 11 changed files with 184 additions and 62 deletions.
diff --git a/.changeset/great-owls-juggle.md 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
@@ -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
@@ -4200,7 +4200,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(`<!DOCTYPE html>`, { url: "https://remix.run/" });
+  const dom = new JSDOM(`<!DOCTYPE html>`, { 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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<Path> {
   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,