Don't setTimeout if component is unmounted.#169
Conversation
This happens in the case where the previous fetch hasn't returned yet and the component is unmounted. In that case it would keep refreshing the unmounted component forever. Also includes fix for unrelated non-deterministic test that occurs because the test assumes that two different lines in the test will take place more than one millisecond apart, which is not always true.
| // force the refresh to happen now after it has been unmounted | ||
| after() | ||
|
|
||
| const spy = expect.spyOn(window, 'setTimeout') |
There was a problem hiding this comment.
shouldn't this happen before after()? I'm also not really clear what's going on there with the spy.destroy() below..
There was a problem hiding this comment.
Hmm.. You're right that it is strange that it is after but I was following the 'should not call setState if component is unmounted' test which does it similarly. But it seems to be working like this because it fails without my change and passes with it.
I was having a hard time coming up with a better test than this. If you have any ideas that would be great.
There was a problem hiding this comment.
I just tested this (and the other test) with the spy up before after() and without the spy.destroy() and it worked fine. The reason it didn't really matter is that the setImmediate is what triggers the change by yielding to the promise. I'll merge this as is and then fix both of the tests.
| outerComponent.setRender(false) | ||
|
|
||
| // force the refresh to happen now after it has been unmounted | ||
| after() |
There was a problem hiding this comment.
can you explain what this is doing? it appears that it's just forcing the call the the refreshTimeout._onTimeout, but doesn't seem like that would actually test the code you added.
There was a problem hiding this comment.
I'm forcing the refreshTimeout._onTimeout after the component has been unmounted. If _onTimeout is called without the setRender(false), the setTimeout gets called and the refresh continues indefinitely even though the component has unmounted.
If you remove the code I added, this test does indeed fail. I'm not sure this is the proper fix, but it seems to be working for the issues I've been having.
|
Thanks for the fix! |
|
Pre-released in v1.0.1-0. |
2 participants
Fixes #166
This happens in the case where the previous fetch hasn't resolved yet
and the component is unmounted. In that case it keeps refreshing the
unmounted component forever.
Also includes fix for unrelated non-deterministic test that occurs because
the test assumes that two different lines in the test will take place more
than one millisecond apart, which is not always true.