Repository: remix-run/remix
Pull request #2879 (2022-04-19 19:35:50)

feat: Allow pass-through script props in `ScrollRestoration`
We previously updated `<Scripts>` to pass props along to the underlying `<script>` component (https://github.com/remix-run/remix/pull/359). This PR does the same for `<ScrollRestoration>`.

Closes: https://github.com/remix-run/remix/issues/183

- [ ] Docs
- [x] Tests


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: e07b49b89885912394d2e0e638d62c480c905336

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/remix-run/remix/new/chance/scroll-restoration-props?filename=.changeset/popular-needles-float.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Freact%22%3A%20patch%0A---%0A%0Afeat%3A%20Allow%20pass-through%20script%20props%20in%20%60ScrollRestoration%60%0A)



Patch:
diff --git a/.changeset/big-spoons-grab.md b/.changeset/big-spoons-grab.md
new file mode 100644
index 000000000..ab0ddcd25
--- /dev/null
+++ b/.changeset/big-spoons-grab.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": minor
+---
+
+allow defining multiple routes for the same route module file
diff --git a/.changeset/late-files-trade.md b/.changeset/late-files-trade.md
new file mode 100644
index 000000000..275b9a21d
--- /dev/null
+++ b/.changeset/late-files-trade.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+Allow pass-through props to be passed to the script rendered by `ScrollRestoration`
diff --git a/.changeset/thin-oranges-boil.md b/.changeset/thin-oranges-boil.md
new file mode 100644
index 000000000..c3c1017cf
--- /dev/null
+++ b/.changeset/thin-oranges-boil.md
@@ -0,0 +1,15 @@
+---
+"@remix-run/react": patch
+---
+
+Fixed a problem with live reload and firefox infinitely reloading the page
+
+The problem is:
+
+1. Firefox is calling `ws.onclose` immediately upon connecting (?!)
+2. Then we’re trying to reconnect, and upon reconnection, we reload the page.
+3. Firefox then calls `ws.onclose` again after reconnecting and the loop starts over
+
+This fix is to check `event.code === 1006` before actually trying to reconnect and the reload the page. 1006 means the connection was closed abnormally (https://www.rfc-editor.org/rfc/rfc6455#section-7.4.1). In our case, that means the server was shut down in local dev and then the socket can reconnect again when the server is back up.
+
+It’s unclear to me why Firefox is calling `onclose` immediately upon connecting to the web socket, but it does.
diff --git a/contributors.yml b/contributors.yml
index 54b97030a..18c8c6b36 100644
--- a/contributors.yml
+++ b/contributors.yml
@@ -257,6 +257,7 @@
 - lpsinger
 - lswest
 - lucasdibz
+- lucasferreira
 - luispagarcia
 - luisrivas
 - luistak
diff --git a/packages/remix-dev/compiler/assets.ts b/packages/remix-dev/compiler/assets.ts
index af02d8323..60be5b6da 100644
--- a/packages/remix-dev/compiler/assets.ts
+++ b/packages/remix-dev/compiler/assets.ts
@@ -56,10 +56,13 @@ export async function createAssetsManifest(
     config.appDirectory,
     config.entryClientFile
   );
-  let routesByFile: Map<string, Route> = Object.keys(config.routes).reduce(
+  let routesByFile: Map<string, Route[]> = Object.keys(config.routes).reduce(
     (map, key) => {
       let route = config.routes[key];
-      map.set(route.file, route);
+      map.set(
+        route.file,
+        map.has(route.file) ? [...map.get(route.file), route] : [route]
+      );
       return map;
     },
     new Map()
@@ -83,22 +86,27 @@ export async function createAssetsManifest(
         /(^browser-route-module:|\?browser$)/g,
         ""
       );
-      let route = routesByFile.get(entryPointFile);
-      invariant(route, `Cannot get route for entry point ${output.entryPoint}`);
-      let sourceExports = await getRouteModuleExports(config, route.id);
-      routes[route.id] = {
-        id: route.id,
-        parentId: route.parentId,
-        path: route.path,
-        index: route.index,
-        caseSensitive: route.caseSensitive,
-        module: resolveUrl(key),
-        imports: resolveImports(output.imports),
-        hasAction: sourceExports.includes("action"),
-        hasLoader: sourceExports.includes("loader"),
-        hasCatchBoundary: sourceExports.includes("CatchBoundary"),
-        hasErrorBoundary: sourceExports.includes("ErrorBoundary"),
-      };
+      let groupedRoute = routesByFile.get(entryPointFile);
+      invariant(
+        groupedRoute,
+        `Cannot get route(s) for entry point ${output.entryPoint}`
+      );
+      for (let route of groupedRoute) {
+        let sourceExports = await getRouteModuleExports(config, route.id);
+        routes[route.id] = {
+          id: route.id,
+          parentId: route.parentId,
+          path: route.path,
+          index: route.index,
+          caseSensitive: route.caseSensitive,
+          module: resolveUrl(key),
+          imports: resolveImports(output.imports),
+          hasAction: sourceExports.includes("action"),
+          hasLoader: sourceExports.includes("loader"),
+          hasCatchBoundary: sourceExports.includes("CatchBoundary"),
+          hasErrorBoundary: sourceExports.includes("ErrorBoundary"),
+        };
+      }
     }
   }
 
diff --git a/packages/remix-dev/config/routes.ts b/packages/remix-dev/config/routes.ts
index 9d97158d0..7f1e5696e 100644
--- a/packages/remix-dev/config/routes.ts
+++ b/packages/remix-dev/config/routes.ts
@@ -54,6 +54,12 @@ export interface DefineRouteOptions {
    * Should be `true` if this is an index route that does not allow child routes.
    */
   index?: boolean;
+
+  /**
+   * An optional unique id string for this route. Use this if you need to aggregate
+   * two or more routes with the same route file.
+   */
+  id?: string;
 }
 
 interface DefineRouteChildren {
@@ -141,7 +147,7 @@ export function defineRoutes(
       path: path ? path : undefined,
       index: options.index ? true : undefined,
       caseSensitive: options.caseSensitive ? true : undefined,
-      id: createRouteId(file),
+      id: options.id || createRouteId(file),
       parentId:
         parentRoutes.length > 0
           ? parentRoutes[parentRoutes.length - 1].id
@@ -149,6 +155,12 @@ export function defineRoutes(
       file,
     };
 
+    if (route.id in routes) {
+      throw new Error(
+        `Unable to define routes with duplicate route id: "${route.id}"`
+      );
+    }
+
     routes[route.id] = route;
 
     if (children) {
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 45e4a9efc..d953f5172 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -65,7 +65,7 @@ import type {
 ////////////////////////////////////////////////////////////////////////////////
 // RemixEntry
 
-interface RemixEntryContextType {
+export interface RemixEntryContextType {
   manifest: AssetsManifest;
   matches: BaseRouteMatch<ClientRoute>[];
   routeData: RouteData;
@@ -890,7 +890,7 @@ export function Meta() {
  */
 let isHydrated = false;
 
-type ScriptProps = Omit<
+export type ScriptProps = Omit<
   React.HTMLProps<HTMLScriptElement>,
   | "children"
   | "async"
@@ -1646,7 +1646,6 @@ export const LiveReload =
                   let socketPath = protocol + "//" + host + ":" + ${String(
                     port
                   )} + "/socket";
-
                   let ws = new WebSocket(socketPath);
                   ws.onmessage = (message) => {
                     let event = JSON.parse(message.data);
@@ -1663,15 +1662,17 @@ export const LiveReload =
                       config.onOpen();
                     }
                   };
-                  ws.onclose = (error) => {
-                    console.log("Remix dev asset server web socket closed. Reconnecting...");
-                    setTimeout(
-                      () =>
-                        remixLiveReloadConnect({
-                          onOpen: () => window.location.reload(),
-                        }),
-                      1000
-                    );
+                  ws.onclose = (event) => {
+                    if (event.code === 1006) {
+                      console.log("Remix dev asset server web socket closed. Reconnecting...");
+                      setTimeout(
+                        () =>
+                          remixLiveReloadConnect({
+                            onOpen: () => window.location.reload(),
+                          }),
+                        1000
+                      );
+                    }
                   };
                   ws.onerror = (error) => {
                     console.log("Remix dev asset server web socket error:");
diff --git a/packages/remix-react/scroll-restoration.tsx b/packages/remix-react/scroll-restoration.tsx
index b517b4362..3b874a51e 100644
--- a/packages/remix-react/scroll-restoration.tsx
+++ b/packages/remix-react/scroll-restoration.tsx
@@ -2,6 +2,7 @@ import * as React from "react";
 import { useLocation } from "react-router-dom";
 
 import { useBeforeUnload, useTransition } from "./components";
+import type { ScriptProps } from "./components";
 
 let STORAGE_KEY = "positions";
 
@@ -20,7 +21,7 @@ if (typeof document !== "undefined") {
  *
  * @see https://remix.run/api/remix#scrollrestoration
  */
-export function ScrollRestoration({ nonce = undefined }: { nonce?: string }) {
+export function ScrollRestoration(props: ScriptProps) {
   useScrollRestoration();
 
   // wait for the browser to restore it on its own
@@ -54,7 +55,7 @@ export function ScrollRestoration({ nonce = undefined }: { nonce?: string }) {
 
   return (
     <script
-      nonce={nonce}
+      {...props}
       suppressHydrationWarning
       dangerouslySetInnerHTML={{
         __html: `(${restoreScroll})(${JSON.stringify(STORAGE_KEY)})`,
diff --git a/packages/remix-server-runtime/CHANGELOG.md b/packages/remix-server-runtime/CHANGELOG.md
index 99a79cbf7..c76c7bbf7 100644
--- a/packages/remix-server-runtime/CHANGELOG.md
+++ b/packages/remix-server-runtime/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 ### Minor Changes
 
-- We have been busy at work [Layering Remix on top of React Router 6.4](https://github.com/remix-run/remix/blob/main/decisions/0007-remix-on-react-router-6-4-0.md) and are excited to be releasing step 1 in this process that consists of performing all server-side data fetches/mutations through the new framework agnostic `@remix-run/router`. Server- and client-side rendering are still done the same as before, and will be updated in subsequent releases.  ([#4612](https://github.com/remix-run/remix/pull/4612))
+- We have been busy at work [Layering Remix on top of React Router 6.4](https://github.com/remix-run/remix/blob/main/decisions/0007-remix-on-react-router-6-4-0.md) and are excited to be releasing step 1 in this process that consists of performing all server-side data fetches/mutations through the new framework agnostic `@remix-run/router`. Server- and client-side rendering are still done the same as before, and will be updated in subsequent releases. ([#4612](https://github.com/remix-run/remix/pull/4612))
 - Importing functions and types from the `remix` package is deprecated, and all ([#3284](https://github.com/remix-run/remix/pull/3284))
   exported modules will be removed in the next major release. For more details,
   [see the release notes for 1.4.0](https://github.com/remix-run/remix/releases/tag/v1.4.0)


Test patch:
diff --git a/packages/remix-dev/__tests__/defineRoutes-test.ts b/packages/remix-dev/__tests__/defineRoutes-test.ts
index d4766d004..dbd73350d 100644
--- a/packages/remix-dev/__tests__/defineRoutes-test.ts
+++ b/packages/remix-dev/__tests__/defineRoutes-test.ts
@@ -87,4 +87,80 @@ describe("defineRoutes", () => {
       }
     `);
   });
+
+  it("allows multiple routes with the same route module", () => {
+    let routes = defineRoutes((route) => {
+      route("/user/:id", "routes/index.tsx", { id: "user-by-id" });
+      route("/user", "routes/index.tsx", { id: "user" });
+      route("/other", "routes/other-route.tsx");
+    });
+
+    expect(routes).toMatchInlineSnapshot(`
+      Object {
+        "routes/other-route": Object {
+          "caseSensitive": undefined,
+          "file": "routes/other-route.tsx",
+          "id": "routes/other-route",
+          "index": undefined,
+          "parentId": undefined,
+          "path": "/other",
+        },
+        "user": Object {
+          "caseSensitive": undefined,
+          "file": "routes/index.tsx",
+          "id": "user",
+          "index": undefined,
+          "parentId": undefined,
+          "path": "/user",
+        },
+        "user-by-id": Object {
+          "caseSensitive": undefined,
+          "file": "routes/index.tsx",
+          "id": "user-by-id",
+          "index": undefined,
+          "parentId": undefined,
+          "path": "/user/:id",
+        },
+      }
+    `);
+  });
+
+  it("throws an error on route id collisions", () => {
+    // Two conflicting custom id's
+    let defineNonUniqueRoutes = () => {
+      defineRoutes((route) => {
+        route("/user/:id", "routes/user.tsx", { id: "user" });
+        route("/user", "routes/user.tsx", { id: "user" });
+        route("/other", "routes/other-route.tsx");
+      });
+    };
+
+    expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
+      `"Unable to define routes with duplicate route id: \\"user\\""`
+    );
+
+    // Custom id conflicting with a later-defined auto-generated id
+    defineNonUniqueRoutes = () => {
+      defineRoutes((route) => {
+        route("/user/:id", "routes/user.tsx", { id: "routes/user" });
+        route("/user", "routes/user.tsx");
+      });
+    };
+
+    expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
+      `"Unable to define routes with duplicate route id: \\"routes/user\\""`
+    );
+
+    // Custom id conflicting with an earlier-defined auto-generated id
+    defineNonUniqueRoutes = () => {
+      defineRoutes((route) => {
+        route("/user", "routes/user.tsx");
+        route("/user/:id", "routes/user.tsx", { id: "routes/user" });
+      });
+    };
+
+    expect(defineNonUniqueRoutes).toThrowErrorMatchingInlineSnapshot(
+      `"Unable to define routes with duplicate route id: \\"routes/user\\""`
+    );
+  });
 });
diff --git a/packages/remix-react/__tests__/scroll-restoration-test.tsx b/packages/remix-react/__tests__/scroll-restoration-test.tsx
new file mode 100644
index 000000000..bc4ae14d1
--- /dev/null
+++ b/packages/remix-react/__tests__/scroll-restoration-test.tsx
@@ -0,0 +1,104 @@
+import * as React from "react";
+import { MemoryRouter, Outlet } from "react-router-dom";
+import { render, screen } from "@testing-library/react";
+
+import { LiveReload, RemixEntryContext, Scripts } from "../components";
+import type { RemixEntryContextType } from "../components";
+import { ScrollRestoration } from "../scroll-restoration";
+
+import "@testing-library/jest-dom/extend-expect";
+
+function AppShell({ children }: { children: React.ReactNode }) {
+  return (
+    <React.Fragment>
+      <Outlet />
+      {children}
+      <Scripts />
+      <LiveReload />
+    </React.Fragment>
+  );
+}
+
+describe("<ScrollRestoration />", () => {
+  function withContext(stuff: JSX.Element) {
+    let context: RemixEntryContextType = {
+      routeModules: { idk: { default: () => null } },
+      manifest: {
+        routes: {
+          idk: {
+            hasLoader: true,
+            hasAction: false,
+            hasCatchBoundary: false,
+            hasErrorBoundary: false,
+            id: "idk",
+            module: "idk",
+          },
+        },
+        entry: { imports: [], module: "" },
+        url: "",
+        version: "",
+      },
+      matches: [],
+      clientRoutes: [
+        {
+          id: "idk",
+          path: "idk",
+          hasLoader: true,
+          element: "",
+          module: "",
+          async action() {
+            return {};
+          },
+          async loader() {
+            return {};
+          },
+        },
+      ],
+      routeData: {},
+      appState: {} as any,
+      transitionManager: {
+        getState() {
+          return {
+            transition: {},
+          };
+        },
+      } as any,
+    };
+    return (
+      <RemixEntryContext.Provider value={context}>
+        <MemoryRouter>{stuff}</MemoryRouter>
+      </RemixEntryContext.Provider>
+    );
+  }
+
+  it("should render a <script> tag", () => {
+    render(
+      withContext(
+        <AppShell>
+          <ScrollRestoration data-testid="scroll-script" />
+        </AppShell>
+      )
+    );
+    let script = screen.getByTestId("scroll-script");
+    expect(script instanceof HTMLScriptElement).toBe(true);
+  });
+
+  it("should pass props to <script>", () => {
+    render(
+      withContext(
+        <AppShell>
+          <ScrollRestoration
+            data-testid="scroll-script"
+            nonce="hello"
+            crossOrigin="anonymous"
+          />
+        </AppShell>
+      )
+    );
+    let script = screen.getByTestId("scroll-script");
+    expect(script).toHaveAttribute("nonce", "hello");
+    expect(script).toHaveAttribute("crossorigin", "anonymous");
+  });
+
+  it.todo("should restore scroll position");
+});