Fix: make v5 Router compatible with v18 StrictMode (#8831)

remix-run · May 9, 2022 · a68510f · a68510f
1 parent d8098b6
commit a68510f
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 5 deletions.
diff --git a/packages/react-router/modules/Router.js b/packages/react-router/modules/Router.js
@@ -30,18 +30,26 @@ class Router extends React.Component {
 
     if (!props.staticContext) {
       this.unlisten = props.history.listen(location => {
-        if (this._isMounted) {
-          this.setState({ location });
-        } else {
-          this._pendingLocation = location;
-        }
+        this._pendingLocation = location;
       });
     }
   }
 
   componentDidMount() {
     this._isMounted = true;
 
+    if (this.unlisten) {
+      // Any pre-mount location changes have been captured at
+      // this point, so unregister the listener.
+      this.unlisten();
+    }
+    if (!this.props.staticContext) {
+      this.unlisten = this.props.history.listen(location => {
+        if (this._isMounted) {
+          this.setState({ location });
+        }
+      });
+    }
     if (this._pendingLocation) {
       this.setState({ location: this._pendingLocation });
     }

diff --git a/packages/react-router/modules/__tests__/Router-test.js b/packages/react-router/modules/__tests__/Router-test.js
@@ -46,4 +46,66 @@ describe("A <Router>", () => {
       }).not.toThrow();
     });
   });
+
+  describe("with react v18 StrictMode semantics", () => {
+    function simulateV18StrictModeRender(props, afterRender) {
+      // Constructor gets called twice
+      let router = new Router(props);
+      router = new Router(props);
+
+      // Swap out setState since we're not actually mounting the component
+      router.setState = jest.fn();
+
+      // Render is called twice
+      router.render();
+      router.render();
+
+      if (afterRender) afterRender();
+
+      router.componentDidMount();
+
+      // Effects are invoked twice
+      router.componentWillUnmount();
+      router.componentDidMount();
+
+      return router;
+    }
+
+    it("responds to history changes", () => {
+      const history = createHistory();
+      const props = { history, children: <p>Foo</p> };
+
+      const router = simulateV18StrictModeRender(props);
+
+      history.push("/foo");
+
+      expect(router.setState).toHaveBeenCalledTimes(1);
+      expect(router.setState).toHaveBeenCalledWith({
+        location: expect.objectContaining({ pathname: "/foo" })
+      });
+    });
+
+    it("can handle path changes on child mount", () => {
+      const history = createHistory();
+      const props = { history, children: <p>Foo</p> };
+
+      const router = simulateV18StrictModeRender(props, () => {
+        history.push("/baz");
+        history.push("/qux");
+        history.push("/foo");
+      });
+
+      expect(router.setState).toHaveBeenCalledTimes(1);
+      expect(router.setState).toHaveBeenCalledWith({
+        location: expect.objectContaining({ pathname: "/foo" })
+      });
+
+      history.push("/bar");
+
+      expect(router.setState).toHaveBeenCalledTimes(2);
+      expect(router.setState).toHaveBeenCalledWith({
+        location: expect.objectContaining({ pathname: "/bar" })
+      });
+    });
+  });
 });