prevent reload of page if an error occurs in onClick event handler (#…

…6711) * prevent reload of page if an error occurs in onClick event handler in <Link /> 1. catch exception of onClick 2. prevent default event handler 3. re-throw exception * add tests * remove unused Component import in <Link /> test
remix-run · Apr 20, 2019 · 82ce94c · 82ce94c
1 parent 7bd1407
commit 82ce94c
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 19 deletions.
diff --git a/packages/react-router-dom/.size-snapshot.json b/packages/react-router-dom/.size-snapshot.json
@@ -1,8 +1,8 @@
 {
   "esm/react-router-dom.js": {
-    "bundled": 8076,
-    "minified": 4865,
-    "gzipped": 1626,
+    "bundled": 8159,
+    "minified": 4903,
+    "gzipped": 1641,
     "treeshaked": {
       "rollup": {
         "code": 453,
@@ -14,13 +14,13 @@
     }
   },
   "umd/react-router-dom.js": {
-    "bundled": 158978,
-    "minified": 56593,
-    "gzipped": 16361
+    "bundled": 159106,
+    "minified": 56622,
+    "gzipped": 16367
   },
   "umd/react-router-dom.min.js": {
-    "bundled": 95833,
-    "minified": 33613,
-    "gzipped": 9925
+    "bundled": 95961,
+    "minified": 33642,
+    "gzipped": 9929
   }
 }
diff --git a/packages/react-router-dom/modules/Link.js b/packages/react-router-dom/modules/Link.js
@@ -13,7 +13,12 @@ function isModifiedEvent(event) {
  */
 class Link extends React.Component {
   handleClick(event, history) {
-    if (this.props.onClick) this.props.onClick(event);
+    try {
+      if (this.props.onClick) this.props.onClick(event);
+    } catch (ex) {
+      event.preventDefault();
+      throw ex;
+    }
 
     if (
       !event.defaultPrevented && // onClick prevented default

diff --git a/packages/react-router-dom/modules/__tests__/Link-test.js b/packages/react-router-dom/modules/__tests__/Link-test.js
@@ -276,5 +276,42 @@ describe("A <Link>", () => {
 
       expect(memoryHistory.push).toBeCalledTimes(0);
     });
+
+    it("prevents the default event handler if an error occurs", () => {
+      const memoryHistory = createMemoryHistory();
+      memoryHistory.push = jest.fn();
+      const error = new Error();
+      const clickHandler = () => {
+        throw error;
+      };
+      const mockPreventDefault = jest.fn();
+      const to = "/the/path?the=query#the-hash";
+
+      renderStrict(
+        <Router history={memoryHistory}>
+          <Link to={to} onClick={clickHandler}>
+            link
+          </Link>
+        </Router>,
+        node
+      );
+
+      console.error = jest.fn(); // keep console clean. Dunno why the catch doesn't do the job correctly.
+      try {
+        const a = node.querySelector("a");
+        ReactTestUtils.Simulate.click(a, {
+          defaultPrevented: false,
+          preventDefault: mockPreventDefault,
+          button: 1
+        });
+      } catch (e) {
+        expect(e).toBe(error);
+      }
+
+      console.error.mockRestore();
+      expect(clickHandler).toThrow(error);
+      expect(mockPreventDefault).toHaveBeenCalled();
+      expect(memoryHistory.push).toBeCalledTimes(0);
+    });
   });
 });
diff --git a/packages/react-router/.size-snapshot.json b/packages/react-router/.size-snapshot.json
@@ -1,8 +1,8 @@
 {
   "esm/react-router.js": {
-    "bundled": 23396,
-    "minified": 13250,
-    "gzipped": 3680,
+    "bundled": 23435,
+    "minified": 13241,
+    "gzipped": 3679,
     "treeshaked": {
       "rollup": {
         "code": 2214,
@@ -14,13 +14,13 @@
     }
   },
   "umd/react-router.js": {
-    "bundled": 98991,
-    "minified": 35022,
-    "gzipped": 11227
+    "bundled": 99032,
+    "minified": 35013,
+    "gzipped": 11222
   },
   "umd/react-router.min.js": {
-    "bundled": 61594,
-    "minified": 21423,
-    "gzipped": 7606
+    "bundled": 61635,
+    "minified": 21414,
+    "gzipped": 7600
   }
 }