Fix router re-initialization bug

contributors.yml

@@ -213,6 +213,7 @@
 - souzasmatheus
 - srmagura
 - stasundr
+- steinybot
 - stmtk1
 - swalker326
 - tanayv

packages/react-router/__tests__/gh-issue-11300-test.tsx

@@ -0,0 +1,137 @@
+import * as React from "react";
+import type {
+  Dispatch,
+  ReactNode,
+  SetStateAction} from "react";
+import {
+  createContext,
+  useContext,
+  useState,
+} from "react";
+import {
+  createMemoryRouter,
+  createRoutesFromElements,
+  Route,
+  RouterProvider,
+  useLoaderData,
+} from "react-router";
+import {
+  act,
+  fireEvent,
+  getByRole,
+  getByText,
+  render,
+  waitFor,
+} from "@testing-library/react";
+import { createDeferred } from "../../router/__tests__/utils/utils";
+
+type Environment = {
+  id: number;
+};
+
+type Query = {
+  environment: Environment;
+};
+
+type State<A> = [A, Dispatch<SetStateAction<A>>];
+
+describe("GH Issue #11300", () => {
+  it("works", async () => {
+    const EnvironmentContext = createContext<State<Environment> | undefined>(
+      undefined
+    );
+
+    function useEnvironment(): Environment {
+      const environmentState = useContext(EnvironmentContext);
+      if (environmentState === undefined)
+        throw new Error("EnvironmentContext.Provider is missing");
+      return environmentState[0];
+    }
+
+    let environmentId = 0;
+
+    function createEnvironment(): Environment {
+      return {
+        id: ++environmentId,
+      };
+    }
+
+    function useCreateNewEnvironment(): () => void {
+      const environmentState = useContext(EnvironmentContext);
+      if (environmentState === undefined)
+        throw new Error("EnvironmentContext.Provider is missing");
+      return () => environmentState[1](createEnvironment());
+    }
+
+    function MutableEnvironment({ children }: { children: ReactNode }) {
+      const environmentState = useState<Environment>(createEnvironment);
+      return (
+        <EnvironmentContext.Provider value={environmentState}>
+          {children}
+        </EnvironmentContext.Provider>
+      );
+    }
+
+    function loadQuery(environment: Environment): Query {
+      return { environment };
+    }
+
+    function useQuery(query: Query): string {
+      const environment = useEnvironment();
+      if (query.environment.id !== environment.id)
+        throw new Error(
+          "Query environment does not match environment from the context"
+        );
+      return `Data... (environment.id = ${environment.id})`;
+    }
+
+    function MyPage() {
+      const query = useLoaderData() as Query;
+      const data = useQuery(query);
+      const createNewEnvironment = useCreateNewEnvironment();
+      return (
+        <>
+          <p>{data}</p>
+          <button onClick={createNewEnvironment}>Create new environment</button>
+        </>
+      );
+    }
+
+    function MyRouter() {
+      const environment = useEnvironment();
+      let router = createMemoryRouter(
+        createRoutesFromElements(
+          <Route
+            path="/"
+            loader={() => loadQuery(environment)}
+            element={<MyPage />}
+          />
+        )
+      );
+      return <RouterProvider router={router} />;
+    }
+
+    let { container } = render(
+      <MutableEnvironment>
+        <MyRouter />
+      </MutableEnvironment>
+    );
+
+    // Somehow this avoids:
+    //  Warning: An update to RouterProvider inside a test was not wrapped in act(...).
+    // I suspect that it has something to do with the way completeNavigation runs
+    // asynchronously and will yield to render.
+    let fooDefer = createDeferred();
+    await act(() => fooDefer.resolve(123));
+
+    expect(getByText(container, "Data... (environment.id = 1)")).toBeDefined();
+
+    fireEvent.click(getByRole(container, "button"));
+
+    await waitFor(() => {
+      expect(
+        getByText(container, "Data... (environment.id = 2)")
+      ).toBeDefined();
+    });
+  });
+});

packages/react-router/__tests__/tsconfig.json

@@ -0,0 +1,4 @@
+{
+  "extends": "../tsconfig.json",
+  "include": ["*"],
+}

packages/react-router/lib/components.tsx

@@ -14,14 +14,15 @@ import {
   AbortedDeferredError,
   Action as NavigationType,
   createMemoryHistory,
-  UNSAFE_getResolveToMatches as getResolveToMatches,
-  UNSAFE_invariant as invariant,
   parsePath,
   resolveTo,
   stripBasename,
+  UNSAFE_getResolveToMatches as getResolveToMatches,
+  UNSAFE_invariant as invariant,
   UNSAFE_warning as warning,
 } from "@remix-run/router";
 import * as React from "react";
+import { useEffect, useRef } from "react";
 
 import type {
   DataRouteObject,
@@ -64,26 +65,26 @@ export interface RouterProviderProps {
 }
 
 /**
-  Webpack + React 17 fails to compile on any of the following because webpack
-  complains that `startTransition` doesn't exist in `React`:
-  * import { startTransition } from "react"
-  * import * as React from from "react";
-    "startTransition" in React ? React.startTransition(() => setState()) : setState()
-  * import * as React from from "react";
-    "startTransition" in React ? React["startTransition"](() => setState()) : setState()
-
-  Moving it to a constant such as the following solves the Webpack/React 17 issue:
-  * import * as React from from "react";
-    const START_TRANSITION = "startTransition";
-    START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState()
-
-  However, that introduces webpack/terser minification issues in production builds
-  in React 18 where minification/obfuscation ends up removing the call of
-  React.startTransition entirely from the first half of the ternary.  Grabbing
-  this exported reference once up front resolves that issue.
-
-  See https://github.com/remix-run/react-router/issues/10579
-*/
+ Webpack + React 17 fails to compile on any of the following because webpack
+ complains that `startTransition` doesn't exist in `React`:
+ * import { startTransition } from "react"
+ * import * as React from from "react";
+ "startTransition" in React ? React.startTransition(() => setState()) : setState()
+ * import * as React from from "react";
+ "startTransition" in React ? React["startTransition"](() => setState()) : setState()
+
+ Moving it to a constant such as the following solves the Webpack/React 17 issue:
+ * import * as React from from "react";
+ const START_TRANSITION = "startTransition";
+ START_TRANSITION in React ? React[START_TRANSITION](() => setState()) : setState()
+
+ However, that introduces webpack/terser minification issues in production builds
+ in React 18 where minification/obfuscation ends up removing the call of
+ React.startTransition entirely from the first half of the ternary.  Grabbing
+ this exported reference once up front resolves that issue.
+
+ See https://github.com/remix-run/react-router/issues/10579
+ */
 const START_TRANSITION = "startTransition";
 const startTransitionImpl = React[START_TRANSITION];
 
@@ -154,6 +155,14 @@ export function RouterProvider({
     [router, navigator, basename]
   );
 
+  const wasRouterPreviouslyInitialized = useRef(false);
+  const isRouterInitialized = router.state.initialized;
+  const routerReinitializing =
+    wasRouterPreviouslyInitialized && !isRouterInitialized;
+  useEffect(() => {
+    wasRouterPreviouslyInitialized.current = isRouterInitialized;
+  }, [wasRouterPreviouslyInitialized, isRouterInitialized]);
+
   // The fragment and {null} here are important!  We need them to keep React 18's
   // useId happy when we are server-rendering since we may have a <script> here
   // containing the hydrated server-side staticContext (from StaticRouterProvider).
@@ -173,7 +182,7 @@ export function RouterProvider({
               v7_relativeSplatPath: router.future.v7_relativeSplatPath,
             }}
           >
-            {state.initialized || router.future.v7_partialHydration ? (
+            {(state.initialized && !routerReinitializing) || router.future.v7_partialHydration ? (
               <DataRoutes
                 routes={router.routes}
                 future={router.future}