From e549c2bcd837d952b0c9b440a4d74e664c930c0c Mon Sep 17 00:00:00 2001
From: JeremyHolcomb <jholcomb@kyriba.com>
Date: Wed, 24 Apr 2019 07:09:46 -0700
Subject: [PATCH] Fix setState call after unmount

---
 src/minimalReactRouter.test.js  | 24 +++++++++++++++++++++++-
 src/minimalReactRouter.ts       |  8 ++++++--
 src/types.ts                    |  1 +
 src/utils/resolveRouteAction.ts | 13 +++++++++----
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/src/minimalReactRouter.test.js b/src/minimalReactRouter.test.js
index 35c1661..9e216d8 100644
--- a/src/minimalReactRouter.test.js
+++ b/src/minimalReactRouter.test.js
@@ -1,6 +1,8 @@
+/* eslint-disable no-console */
 import React from "react";
+import ReactDOM from "react-dom";
 import { createRouter } from "./minimalReactRouter";
-import { cleanup, render, waitForElement } from "react-testing-library";
+import { cleanup, render, waitForElement, wait } from "react-testing-library";
 
 afterEach(cleanup);
 
@@ -51,3 +53,23 @@ test("can return result from multiple routes", async () => {
     </DocumentFragment>
   `);
 });
+
+test("does not call setState on unmounted route", async () => {
+  const router = createRouter(window.history, "/foo");
+  const routes = { "/foo": () => <div>foo</div> };
+  function Foo() {
+    const [component = null] = router.useRoutes(routes);
+    return component;
+  }
+  const { container } = render(<Foo />);
+  console.error = jest.fn(console.error);
+  ReactDOM.unmountComponentAtNode(container);
+  await wait(() => {
+    expect(
+      console.error.mock.calls.find(([message]) =>
+        message.includes("unmounted component")
+      )
+    ).toBe(undefined);
+    console.error.mockRestore();
+  });
+});
diff --git a/src/minimalReactRouter.ts b/src/minimalReactRouter.ts
index 66699f4..5f4f12b 100644
--- a/src/minimalReactRouter.ts
+++ b/src/minimalReactRouter.ts
@@ -32,6 +32,7 @@ const [setRoute] = resolveLatest(async function setRoute(
   const { routeResolvers } = internalState;
   let anyMatch = false;
   for (let resolverKey of Object.getOwnPropertySymbols(routeResolvers)) {
+    internalState.currentRouteToken = resolverKey;
     const resolveRoute = routeResolvers[(resolverKey as unknown) as string];
     if (resolveRoute) {
       const status = await resolveRoute(url);
@@ -75,6 +76,7 @@ function useRoutesFn(
   }, [routeResolvers, resolveRoute, resolverKey]);
   // Resolve initial route when component mounts.
   useEffect(() => {
+    internalState.currentRouteToken = resolverKey;
     resolveRoute(internalState.url);
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
@@ -84,7 +86,8 @@ function useRoutesFn(
 
 export function createRouter(
   urlHistory: URLHistory,
-  initialURL: string
+  initialURL: string,
+  defaultResult?: any
 ): Router {
   if (
     !urlHistory ||
@@ -97,10 +100,11 @@ export function createRouter(
     throw new TypeError(`Failed to create router: invalid inital URL`);
   }
   const internalState: RouterInternalState = {
+    currentRouteToken: Symbol(),
     initialState: {
       parameters: [],
       path: new PathURL(initialURL),
-      result: undefined
+      result: defaultResult
     },
     redirectStack: [],
     routeResolvers: {},
diff --git a/src/types.ts b/src/types.ts
index e7e769e..ae88530 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -32,6 +32,7 @@ export interface Routes {
 }
 
 export interface RouterInternalState {
+  currentRouteToken: symbol;
   initialState: ResolvedRoute;
   redirectStack: string[];
   routeResolvers: Record<string, ((url: string) => Promise<boolean>) | null> &
diff --git a/src/utils/resolveRouteAction.ts b/src/utils/resolveRouteAction.ts
index 52308c0..baca1a3 100644
--- a/src/utils/resolveRouteAction.ts
+++ b/src/utils/resolveRouteAction.ts
@@ -15,6 +15,7 @@ export async function resolveRouteAction(
   routerReplace: RouterMethod,
   url: string
 ): Promise<boolean> {
+  const { currentRouteToken, redirectStack, routeResolvers } = internalState;
   const pathURL = new PathURL(url);
   const matchedRoute = processedRoutes.find(({ regExpPath }) =>
     regExpPath.test(pathURL.ensureFinalSlash())
@@ -24,10 +25,10 @@ export async function resolveRouteAction(
   }
   let redirectPromise: Promise<void> | null = null;
   const redirect = (redirectURL: string): Promise<void> => {
-    internalState.redirectStack.push(url);
-    if (internalState.redirectStack.length >= 10) {
+    redirectStack.push(url);
+    if (redirectStack.length >= 10) {
       throw new Error(
-        `Exceeded redirect limit.\n${internalState.redirectStack.join(" -> ")}`
+        `Exceeded redirect limit.\n${redirectStack.join(" -> ")}`
       );
     }
     redirectPromise = routerReplace(redirectURL);
@@ -42,6 +43,10 @@ export async function resolveRouteAction(
   if (redirectPromise !== null) {
     return redirectPromise;
   }
+  // Component will unmount or has unmounted.
+  if (!routeResolvers[(currentRouteToken as unknown) as string]) {
+    return false;
+  }
   const prevState = getState();
   const isSameState =
     Object.is(result, prevState.result) &&
@@ -51,6 +56,6 @@ export async function resolveRouteAction(
     setState({ parameters, path: pathURL, result });
   }
   internalState.url = url;
-  internalState.redirectStack.length = 0;
+  redirectStack.length = 0;
   return true;
 }