Skip to content

Conversation

@MeiKatz
Copy link
Contributor

@MeiKatz MeiKatz commented Apr 18, 2019

Solves: #6709

  1. catch exception of onClick
  2. prevent default event handler
  3. re-throw exception

… <Link />

1. catch exception of onClick
2. prevent default event handler
3. re-throw exception
@timdorr
Copy link
Member

timdorr commented Apr 18, 2019

Perfect. Can we add a test?

@StringEpsilon
Copy link
Contributor

@MeiKatz Unit test:

it("prevents default on exception in onClick callback", () => {
  const memoryHistory = createMemoryHistory();
  memoryHistory.push = jest.fn();
  const onClick = () => { throw new Error };
  const mockPreventDefault = jest.fn();

  renderStrict(
    <Router history={memoryHistory}>
      <Link to="/foo/bar" onClick={onClick}>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) {
    // nothing to do.
  }

  console.error.mockRestore();
  expect(mockPreventDefault).toHaveBeenCalled();
  expect(memoryHistory.push).toBeCalledTimes(0);
})

@MeiKatz
Copy link
Contributor Author

MeiKatz commented Apr 20, 2019

@StringEpsilon I have tried something similar to yours with an error boundary but was stuck at the same problem like you: why does console.error print an error even so I catch it? Nevertheless, I merged the best of your test and mine together 😄

@timdorr
Copy link
Member

timdorr commented Apr 20, 2019

That works for me. Thanks!

@timdorr timdorr merged commit 82ce94c into remix-run:master Apr 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants