Repository: remix-run/remix
Pull request #3929 (2022-08-05 11:13:10)

tests(remix-dev): update to React v18
This shouldn't have been done in #3917, as `package.json` is updated to React v18

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

Latest commit: 2370829f5dac5f009428006bda58a40fdb94658b

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/MichaelDeBoey/remix/new/update-tests-to-react-18?filename=.changeset/lucky-stingrays-work.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Atests(remix-dev)%3A%20update%20to%20React%20v18%0A)



Patch:
diff --git a/.changeset/purple-ligers-drive.md b/.changeset/purple-ligers-drive.md
new file mode 100644
index 000000000..51ac8ce48
--- /dev/null
+++ b/.changeset/purple-ligers-drive.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/dev": patch
+---
+
+Make MDX builds deterministic
diff --git a/.changeset/slow-apples-pretend.md b/.changeset/slow-apples-pretend.md
new file mode 100644
index 000000000..7547330d3
--- /dev/null
+++ b/.changeset/slow-apples-pretend.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+Add subscribe method to transition manager to allow subscribing and un-subscribing for React 18 strict mode compliance.
diff --git a/integration/helpers/node-template/app/entry.client.tsx b/integration/helpers/node-template/app/entry.client.tsx
index d7ef6febb..ffef95f66 100644
--- a/integration/helpers/node-template/app/entry.client.tsx
+++ b/integration/helpers/node-template/app/entry.client.tsx
@@ -1,4 +1,4 @@
-import { hydrateRoot } from "react-dom/client";
 import { RemixBrowser } from "@remix-run/react";
+import { hydrateRoot } from "react-dom/client";
 
 hydrateRoot(document, <RemixBrowser />);
diff --git a/integration/helpers/node-template/app/entry.server.tsx b/integration/helpers/node-template/app/entry.server.tsx
index 2a3e66789..92ac9fc51 100644
--- a/integration/helpers/node-template/app/entry.server.tsx
+++ b/integration/helpers/node-template/app/entry.server.tsx
@@ -1,8 +1,8 @@
 import { PassThrough } from "stream";
-import { renderToPipeableStream } from "react-dom/server";
 import type { EntryContext } from "@remix-run/node";
 import { Response } from "@remix-run/node";
 import { RemixServer } from "@remix-run/react";
+import { renderToPipeableStream } from "react-dom/server";
 
 const ABORT_DELAY = 5000;
 
@@ -18,28 +18,31 @@ export default function handleRequest(
     let { pipe, abort } = renderToPipeableStream(
       <RemixServer context={remixContext} url={request.url} />,
       {
-        onShellReady() {
+        onShellReady: () => {
           let body = new PassThrough();
 
           responseHeaders.set("Content-Type", "text/html");
 
           resolve(
             new Response(body, {
-              status: didError ? 500 : responseStatusCode,
               headers: responseHeaders,
+              status: didError ? 500 : responseStatusCode,
             })
           );
+
           pipe(body);
         },
-        onShellError(err) {
+        onShellError: (err) => {
           reject(err);
         },
-        onError(error) {
+        onError: (error) => {
           didError = true;
+
           console.error(error);
         },
       }
     );
+
     setTimeout(abort, ABORT_DELAY);
   });
 }
diff --git a/packages/remix-dev/compiler/plugins/mdx.ts b/packages/remix-dev/compiler/plugins/mdx.ts
index 2b0ec22e8..4ff210ec9 100644
--- a/packages/remix-dev/compiler/plugins/mdx.ts
+++ b/packages/remix-dev/compiler/plugins/mdx.ts
@@ -39,14 +39,15 @@ export function mdxPlugin(config: RemixConfig): esbuild.Plugin {
         let resolved = path.resolve(args.resolveDir, resolvedPath);
 
         return {
-          path: resolved,
+          path: path.relative(config.appDirectory, resolved),
           namespace: "mdx",
         };
       });
 
       build.onLoad({ filter: /\.mdx?$/ }, async (args) => {
+        let absolutePath = path.join(config.appDirectory, args.path);
         try {
-          let fileContents = await fsp.readFile(args.path, "utf-8");
+          let fileContents = await fsp.readFile(absolutePath, "utf-8");
 
           let rehypePlugins = [];
           let remarkPlugins = [
@@ -115,7 +116,7 @@ ${remixExports}`;
             errors: errors.length ? errors : undefined,
             warnings: warnings.length ? warnings : undefined,
             contents,
-            resolveDir: path.dirname(args.path),
+            resolveDir: path.dirname(absolutePath),
             loader: getLoaderForFile(args.path),
           };
         } catch (err: any) {
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
index 3a6bef315..4627a44e0 100644
--- a/packages/remix-react/components.tsx
+++ b/packages/remix-react/components.tsx
@@ -49,7 +49,12 @@ import type { RouteMatch as BaseRouteMatch } from "./routeMatching";
 import { matchClientRoutes } from "./routeMatching";
 import type { RouteModules, HtmlMetaDescriptor } from "./routeModules";
 import { createTransitionManager } from "./transition";
-import type { Transition, Fetcher, Submission } from "./transition";
+import type {
+  Transition,
+  TransitionManagerState,
+  Fetcher,
+  Submission,
+} from "./transition";
 
 ////////////////////////////////////////////////////////////////////////////////
 // RemixEntry
@@ -117,20 +122,25 @@ export function RemixEntry({
       catch: entryComponentDidCatchEmulator.catch,
       catchBoundaryId: entryComponentDidCatchEmulator.catchBoundaryRouteId,
       onRedirect: _navigator.replace,
-      onChange: (state) => {
-        setClientState({
-          catch: state.catch,
-          error: state.error,
-          catchBoundaryRouteId: state.catchBoundaryId,
-          loaderBoundaryRouteId: state.errorBoundaryId,
-          renderBoundaryRouteId: null,
-          trackBoundaries: false,
-          trackCatchBoundaries: false,
-        });
-      },
     });
   });
 
+  React.useEffect(() => {
+    let subscriber = (state: TransitionManagerState) => {
+      setClientState({
+        catch: state.catch,
+        error: state.error,
+        catchBoundaryRouteId: state.catchBoundaryId,
+        loaderBoundaryRouteId: state.errorBoundaryId,
+        renderBoundaryRouteId: null,
+        trackBoundaries: false,
+        trackCatchBoundaries: false,
+      });
+    };
+
+    return transitionManager.subscribe(subscriber);
+  }, [transitionManager]);
+
   // Ensures pushes interrupting pending navigations use replace
   // TODO: Move this to React Router
   let navigator: Navigator = React.useMemo(() => {
diff --git a/packages/remix-react/transition.ts b/packages/remix-react/transition.ts
index 346fd5d76..0249db948 100644
--- a/packages/remix-react/transition.ts
+++ b/packages/remix-react/transition.ts
@@ -97,10 +97,13 @@ export interface TransitionManagerInit {
   error?: Error;
   catchBoundaryId?: null | string;
   errorBoundaryId?: null | string;
-  onChange: (state: TransitionManagerState) => void;
   onRedirect: (to: string, state?: any) => void;
 }
 
+export type TransitionManagerSubscriber = (
+  state: TransitionManagerState
+) => void;
+
 export interface Submission {
   action: string;
   method: string;
@@ -422,6 +425,7 @@ export function createTransitionManager(init: TransitionManagerInit) {
   let navigationLoadId = -1;
   let fetchReloadIds = new Map<string, number>();
   let fetchRedirectIds = new Set<string>();
+  let subscribers = new Set<TransitionManagerSubscriber>();
 
   let matches = matchClientRoutes(routes, init.location);
 
@@ -462,7 +466,10 @@ export function createTransitionManager(init: TransitionManagerInit) {
     }
 
     state = Object.assign({}, state, updates);
-    init.onChange(state);
+
+    for (let subscriber of subscribers.values()) {
+      subscriber(state);
+    }
   }
 
   function getState(): TransitionManagerState {
@@ -1382,7 +1389,15 @@ export function createTransitionManager(init: TransitionManagerInit) {
     markFetchersDone(doneKeys);
   }
 
+  function subscribe(subscriber: TransitionManagerSubscriber) {
+    subscribers.add(subscriber);
+    return () => {
+      subscribers.delete(subscriber);
+    };
+  }
+
   return {
+    subscribe,
     send,
     getState,
     getFetcher,


Test patch:
diff --git a/integration/deterministic-build-output-test.ts b/integration/deterministic-build-output-test.ts
index f71371749..8ecf888bb 100644
--- a/integration/deterministic-build-output-test.ts
+++ b/integration/deterministic-build-output-test.ts
@@ -3,11 +3,35 @@ import globby from "globby";
 import fs from "fs";
 import path from "path";
 
-import { createFixtureProject } from "./helpers/create-fixture";
+import { createFixtureProject, js } from "./helpers/create-fixture";
 
 test("builds deterministically under different paths", async () => {
-  let dir1 = await createFixtureProject();
-  let dir2 = await createFixtureProject();
+  // This test validates various flavors of remix virtual modules to ensure
+  // we get identical builds regardless of the parent paths. If a virtual
+  // module resolves or imports from absolute paths (e.g. via `path.resolve`),
+  // the build hashes may change even though the output is identical. This
+  // can cause broken apps (i.e. manifest mismatch) if the server and client
+  // are built separately.
+
+  // Virtual modules tested:
+  //  * browserRouteModulesPlugin (implicitly tested by root route)
+  //  * emptyModulesPlugin (via app/routes/foo.tsx' server import)
+  //  * mdx (via app/routes/index.mdx)
+  //  * serverAssetsManifestPlugin (implicitly tested by build)
+  //  * serverEntryModulePlugin (implicitly tested by build)
+  //  * serverRouteModulesPlugin (implicitly tested by build)
+  let init = {
+    files: {
+      "app/routes/index.mdx": "# hello world",
+      "app/routes/foo.tsx": js`
+        export * from "~/foo/bar.server";
+        export default () => "YAY";
+      `,
+      "app/foo/bar.server.ts": "export const meta = () => []",
+    },
+  };
+  let dir1 = await createFixtureProject(init);
+  let dir2 = await createFixtureProject(init);
 
   expect(dir1).not.toEqual(dir2);
 
diff --git a/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.client.tsx b/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.client.tsx
index a19979b25..ffef95f66 100644
--- a/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.client.tsx
+++ b/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.client.tsx
@@ -1,4 +1,4 @@
-import { hydrate } from "react-dom";
-import { RemixBrowser } from "remix";
+import { RemixBrowser } from "@remix-run/react";
+import { hydrateRoot } from "react-dom/client";
 
-hydrate(<RemixBrowser />, document);
+hydrateRoot(document, <RemixBrowser />);
diff --git a/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.server.tsx b/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.server.tsx
index cae206749..92ac9fc51 100644
--- a/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.server.tsx
+++ b/packages/remix-dev/__tests__/fixtures/replace-remix-imports/app/entry.server.tsx
@@ -1,6 +1,10 @@
-import { renderToString } from "react-dom/server";
-import { RemixServer } from "remix";
-import type { EntryContext } from "remix";
+import { PassThrough } from "stream";
+import type { EntryContext } from "@remix-run/node";
+import { Response } from "@remix-run/node";
+import { RemixServer } from "@remix-run/react";
+import { renderToPipeableStream } from "react-dom/server";
+
+const ABORT_DELAY = 5000;
 
 export default function handleRequest(
   request: Request,
@@ -8,14 +12,37 @@ export default function handleRequest(
   responseHeaders: Headers,
   remixContext: EntryContext
 ) {
-  const markup = renderToString(
-    <RemixServer context={remixContext} url={request.url} />
-  );
+  return new Promise((resolve, reject) => {
+    let didError = false;
+
+    let { pipe, abort } = renderToPipeableStream(
+      <RemixServer context={remixContext} url={request.url} />,
+      {
+        onShellReady: () => {
+          let body = new PassThrough();
+
+          responseHeaders.set("Content-Type", "text/html");
+
+          resolve(
+            new Response(body, {
+              headers: responseHeaders,
+              status: didError ? 500 : responseStatusCode,
+            })
+          );
+
+          pipe(body);
+        },
+        onShellError: (err) => {
+          reject(err);
+        },
+        onError: (error) => {
+          didError = true;
 
-  responseHeaders.set("Content-Type", "text/html");
+          console.error(error);
+        },
+      }
+    );
 
-  return new Response("<!DOCTYPE html>" + markup, {
-    status: responseStatusCode,
-    headers: responseHeaders,
+    setTimeout(abort, ABORT_DELAY);
   });
 }
diff --git a/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.client.tsx b/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.client.tsx
index 3eec1fd0a..ffef95f66 100644
--- a/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.client.tsx
+++ b/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.client.tsx
@@ -1,4 +1,4 @@
 import { RemixBrowser } from "@remix-run/react";
-import { hydrate } from "react-dom";
+import { hydrateRoot } from "react-dom/client";
 
-hydrate(<RemixBrowser />, document);
+hydrateRoot(document, <RemixBrowser />);
diff --git a/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.server.tsx b/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.server.tsx
index 5afa18235..92ac9fc51 100644
--- a/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.server.tsx
+++ b/packages/remix-dev/__tests__/migrations/fixtures/indie-stack/app/entry.server.tsx
@@ -1,6 +1,10 @@
+import { PassThrough } from "stream";
 import type { EntryContext } from "@remix-run/node";
+import { Response } from "@remix-run/node";
 import { RemixServer } from "@remix-run/react";
-import { renderToString } from "react-dom/server";
+import { renderToPipeableStream } from "react-dom/server";
+
+const ABORT_DELAY = 5000;
 
 export default function handleRequest(
   request: Request,
@@ -8,14 +12,37 @@ export default function handleRequest(
   responseHeaders: Headers,
   remixContext: EntryContext
 ) {
-  const markup = renderToString(
-    <RemixServer context={remixContext} url={request.url} />
-  );
+  return new Promise((resolve, reject) => {
+    let didError = false;
+
+    let { pipe, abort } = renderToPipeableStream(
+      <RemixServer context={remixContext} url={request.url} />,
+      {
+        onShellReady: () => {
+          let body = new PassThrough();
+
+          responseHeaders.set("Content-Type", "text/html");
+
+          resolve(
+            new Response(body, {
+              headers: responseHeaders,
+              status: didError ? 500 : responseStatusCode,
+            })
+          );
+
+          pipe(body);
+        },
+        onShellError: (err) => {
+          reject(err);
+        },
+        onError: (error) => {
+          didError = true;
 
-  responseHeaders.set("Content-Type", "text/html");
+          console.error(error);
+        },
+      }
+    );
 
-  return new Response("<!DOCTYPE html>" + markup, {
-    status: responseStatusCode,
-    headers: responseHeaders,
+    setTimeout(abort, ABORT_DELAY);
   });
 }
diff --git a/packages/remix-react/__tests__/transition-test.tsx b/packages/remix-react/__tests__/transition-test.tsx
index c4a37e78b..a0b87ed0e 100644
--- a/packages/remix-react/__tests__/transition-test.tsx
+++ b/packages/remix-react/__tests__/transition-test.tsx
@@ -28,7 +28,6 @@ describe("init", () => {
       actionData: { root: "ACTION DATA" },
       error: new Error("lol"),
       errorBoundaryId: "root",
-      onChange: () => {},
       onRedirect: () => {},
     });
     expect(tm.getState()).toMatchInlineSnapshot(`
@@ -1956,7 +1955,6 @@ function createTestTransitionManager(
     loaderData: { root: "ROOT" },
     location,
     routes: [],
-    onChange() {},
     onRedirect() {},
     ...init,
   });
@@ -2090,11 +2088,11 @@ let setup = ({ url } = { url: "/" }) => {
   ];
 
   let tm = createTestTransitionManager(url, {
-    onChange: handleChange,
     onRedirect: handleRedirect,
     loaderData: { root: "ROOT" },
     routes,
   });
+  tm.subscribe(handleChange);
 
   let navigate_ = (
     location: Location | string,