Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
MeiKatz authored and timdorr committed Apr 20, 2019
1 parent 7bd1407 commit 82ce94c
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 19 deletions.
18 changes: 9 additions & 9 deletions 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,
Expand All @@ -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
}
}
7 changes: 6 additions & 1 deletion packages/react-router-dom/modules/Link.js
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions packages/react-router-dom/modules/__tests__/Link-test.js
Expand Up @@ -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);
});
});
});
18 changes: 9 additions & 9 deletions 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,
Expand All @@ -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
}
}

0 comments on commit 82ce94c

Please sign in to comment.