Expecting result.error after "rerender" is not working

[danielkcz]

• react-hooks-testing-library version: 0.5.0
• react-testing-library version: 7.0.0
• react version: 16.8.6
• node version: 10.12.0
• yarn version: 1.13.0

Relevant code or config:

const { result, rerender } = renderHook(
    ({ second }) => {
        const obj = second ? { foo: "brr", baz: "new" } : { foo: "baz" }
        return useAsObservableSource(obj)
    },
    { initialProps: { second: false } }
)
rerender({ second: true })
expect(result.error).toMatchInlineSnapshot(
    `[Error: the shape of objects passed to useAsObservableSource should be stable]`
)

What you did:

I had to edit the following line and removed the state check to make the test passing.

https://github.com/mpeyper/react-hooks-testing-library/blob/934423c3ffc4b6b546a88fce4b79b1179cc27007/src/index.js#L30

What happened:

Without that modification, the result.error after the rerender is undefined

Reproduction:

Failing test: https://travis-ci.org/mobxjs/mobx-react-lite/builds/533269870#L484
Source code: https://github.com/mobxjs/mobx-react-lite/blob/no-observable-source-failing/test/useAsObservableSource.test.tsx#L344

Problem description:

I am not entirely sure what's happening there. The error is caught within componentDidCatch, but the resultContainer.setError is not called at all. It's really strange.

Suggested solution:

I think that for the purpose of testing the children should be always returned from ErrorBoundary. React will remount it anyway, so there is no problem of endless loop. The state.hasError is more useful in real apps where you want to inform a user about the problem and offer some way out of it.

Edit: Later I realized my change has broken other tests, so it's the not correct way either.

[mpeyper]

I'll take a look when I get to work. I've previously stated (#50) that I'm just going to remove the error boundary as it seems to be causing more issues than it was helping me solve (not catching promises for Suspense).

I'll see if going back to a try/catch makes this work as expected.

[danielkcz]

Yes, you are right, the try/catch fixes this particular scenario perfectly.

I've pushed the changes I've made ☝️ to the fork if you want to base the work on that.

[mpeyper]

I've brought in your changes with a slight adjustment to fix the Suspense tests. I've also added you as a contributor to the README, but if you don't want that let me know and I'll revert that commit.