Repository: remix-run/remix
Pull request #2874 (2022-04-19 17:54:54)

fix: re-inline live reload script
fix: manually set REMIX_DEV_SERVER_WS_PORT in config for users running `remix dev`
fix: read devServerPort from ENV at build time

<!--

👋 Hey, thanks for your interest in contributing to Remix!

If this is a simple docs change, go ahead and delete all this.

**Please ask first before starting work on any significant new features.**

It's never a fun experience to have your pull request declined after investing a
lot of time and effort into a new feature. To avoid this from happening, we
request that contributors create a
[Feature Request discussion](https://github.com/remix-run/remix/discussions/new?category=ideas)
to first discuss any significant new features.

https://github.com/remix-run/remix/blob/main/CONTRIBUTING.md

Please fill in or delete each item below:

-->

Closes: # https://github.com/remix-run/remix/pull/2783#issuecomment-1102169177

- [ ] Docs
- [x] Tests



Patch:
diff --git a/packages/remix-dev/config.ts b/packages/remix-dev/config.ts
index 3214b27bc..8f0cc2b91 100644
--- a/packages/remix-dev/config.ts
+++ b/packages/remix-dev/config.ts
@@ -353,7 +353,14 @@ export async function readConfig(
       path.join("public", "build")
   );
 
-  let devServerPort = await getPort({ port: appConfig.devServerPort || 8002 });
+  if (typeof appConfig.devServerPort !== "number") {
+    appConfig.devServerPort = Number(
+      process.env.REMIX_DEV_SERVER_WS_PORT || 8002
+    );
+  }
+  let devServerPort = await getPort({ port: appConfig.devServerPort });
+  // set env variable so un-bundled servers can use it
+  process.env.REMIX_DEV_SERVER_WS_PORT = `${devServerPort}`;
   let devServerBroadcastDelay = appConfig.devServerBroadcastDelay || 0;
 
   let defaultPublicPath = "/build/";
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index b194d7275..a22655260 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -1392,9 +1392,10 @@ export function useFetchers(): Fetcher[] {
   return [...fetchers.values()];
 }
 
-let liveReloadMounted = false;
 // Dead Code Elimination magic for production builds.
 // This way devs don't have to worry about doing the NODE_ENV check themselves.
+// If running an un-bundled server outside of `remix dev` you will still need
+// to set the REMIX_DEV_SERVER_WS_PORT manually.
 export const LiveReload =
   process.env.NODE_ENV !== "development"
     ? () => null
@@ -1408,36 +1409,40 @@ export const LiveReload =
          */
         nonce?: string;
       }) {
-        let setupLiveReload = (port: number) => {
-          let protocol = location.protocol === "https:" ? "wss:" : "ws:";
-          let host = location.hostname;
-          let socketPath = `${protocol}//${host}:${port}/socket`;
-
-          let ws = new WebSocket(socketPath);
-          ws.onmessage = (message) => {
-            let event = JSON.parse(message.data);
-            if (event.type === "LOG") {
-              console.log(event.message);
-            }
-            if (event.type === "RELOAD") {
-              console.log("💿 Reloading window ...");
-              window.location.reload();
-            }
-          };
-          ws.onerror = (error) => {
-            console.log("Remix dev asset server web socket error:");
-            console.error(error);
-          };
-        };
-
-        React.useEffect(() => {
-          if (!liveReloadMounted) {
-            setupLiveReload(port);
-            liveReloadMounted = true;
-          }
-        }, []);
-
-        return null;
+        let js = String.raw;
+        return (
+          <script
+            nonce={nonce}
+            suppressHydrationWarning
+            dangerouslySetInnerHTML={{
+              __html: js`
+                (() => {
+                  let protocol = location.protocol === "https:" ? "wss:" : "ws:";
+                  let host = location.hostname;
+                  let socketPath = protocol + "//" + host + ":" + ${String(
+                    port
+                  )} + "/socket";
+        
+                  let ws = new WebSocket(socketPath);
+                  ws.onmessage = (message) => {
+                    let event = JSON.parse(message.data);
+                    if (event.type === "LOG") {
+                      console.log(event.message);
+                    }
+                    if (event.type === "RELOAD") {
+                      console.log("💿 Reloading window ...");
+                      window.location.reload();
+                    }
+                  };
+                  ws.onerror = (error) => {
+                    console.log("Remix dev asset server web socket error:");
+                    console.error(error);
+                  };
+                })();
+              `,
+            }}
+          />
+        );
       };
 
 function useComposedRefs<RefValueType = any>(


Test patch:
diff --git a/packages/remix-react/__tests__/components-test.tsx b/packages/remix-react/__tests__/components-test.tsx
index 53226ed97..8df6a1f3a 100644
--- a/packages/remix-react/__tests__/components-test.tsx
+++ b/packages/remix-react/__tests__/components-test.tsx
@@ -2,7 +2,7 @@ import * as React from "react";
 import { MemoryRouter } from "react-router-dom";
 import { fireEvent, render, act } from "@testing-library/react";
 
-// import type { LiveReload as ActualLiveReload } from "../components";
+import type { LiveReload as ActualLiveReload } from "../components";
 import { Link, NavLink, RemixEntryContext } from "../components";
 
 import "@testing-library/jest-dom/extend-expect";
@@ -15,57 +15,58 @@ import "@testing-library/jest-dom/extend-expect";
 // the browser reloads with the new UI. At the moment we could completely break
 // LiveReload's real features and these tests wouldn't know it.
 
-// describe("<LiveReload />", () => {
-//   const originalNodeEnv = process.env.NODE_ENV;
-//   afterEach(() => {
-//     process.env.NODE_ENV = originalNodeEnv;
-//   });
-
-//   describe("non-development environment", () => {
-//     let LiveReload: typeof ActualLiveReload;
-//     beforeEach(() => {
-//       process.env.NODE_ENV = "not-development";
-//       jest.resetModules();
-//       LiveReload = require("../components").LiveReload;
-//     });
-
-//     it("does nothing if the NODE_ENV is not development", () => {
-//       const { container } = render(<LiveReload />);
-//       expect(container).toBeEmptyDOMElement();
-//     });
-//   });
-
-//   describe("development environment", () => {
-//     let LiveReload: typeof ActualLiveReload;
-//     beforeEach(() => {
-//       process.env.NODE_ENV = "development";
-//       jest.resetModules();
-//       LiveReload = require("../components").LiveReload;
-//     });
-
-//     it("defaults the port to 8002", () => {
-//       const { container } = render(<LiveReload />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:8002\/socket/
-//       );
-//     });
-
-//     it("can set the port explicitly", () => {
-//       const { container } = render(<LiveReload port={4321} />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:4321\/socket/
-//       );
-//     });
-
-//     it("determines the right port based on REMIX_DEV_SERVER_WS_PORT env variable", () => {
-//       process.env.REMIX_DEV_SERVER_WS_PORT = "1234";
-//       const { container } = render(<LiveReload />);
-//       expect(container.querySelector("script")).toHaveTextContent(
-//         /:1234\/socket/
-//       );
-//     });
-//   });
-// });
+describe("<LiveReload />", () => {
+  const originalNodeEnv = process.env.NODE_ENV;
+  afterEach(() => {
+    process.env.NODE_ENV = originalNodeEnv;
+  });
+
+  describe("non-development environment", () => {
+    let LiveReload: typeof ActualLiveReload;
+    beforeEach(() => {
+      process.env.NODE_ENV = "not-development";
+      jest.resetModules();
+      LiveReload = require("../components").LiveReload;
+    });
+
+    it("does nothing if the NODE_ENV is not development", () => {
+      const { container } = render(<LiveReload />);
+      expect(container).toBeEmptyDOMElement();
+    });
+  });
+
+  describe("development environment", () => {
+    let oldEnv = process.env;
+    let LiveReload: typeof ActualLiveReload;
+    beforeEach(() => {
+      process.env = { ...oldEnv, NODE_ENV: "development" };
+      jest.resetModules();
+    });
+
+    it("defaults the port to 8002", () => {
+      LiveReload = require("../components").LiveReload;
+      const { container } = render(<LiveReload />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `8002 + "/socket"`
+      );
+    });
+
+    it("can set the port explicitly", () => {
+      const { container } = render(<LiveReload port={4321} />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `4321 + "/socket"`
+      );
+    });
+
+    it("determines the right port based on REMIX_DEV_SERVER_WS_PORT env variable", () => {
+      process.env.REMIX_DEV_SERVER_WS_PORT = "1234";
+      const { container } = render(<LiveReload />);
+      expect(container.querySelector("script")).toHaveTextContent(
+        `1234 + "/socket"`
+      );
+    });
+  });
+});
 
 const setIntentEvents = ["focus", "mouseEnter", "touchStart"] as const;
 type PrefetchEventHandlerProps = {