8000
Skip to content

Add failing test case demonstraiting bad setState() call. - #12

Merged
sheaivey merged 1 commit into
sheaivey:masterfrom
binki:test-unmounted-setstate
Aug 4, 2017
Merged

Add failing test case demonstraiting bad setState() call.#12
sheaivey merged 1 commit into
sheaivey:masterfrom
binki:test-unmounted-setstate

Conversation

@binki
@binki binki commented Jun 12, 2017
Copy link
Copy Markdown
Contributor

This failing test case reproduces the error I reported in #9.

The logic error is that the debounce will call the wrapped function
and the wrapped function does not check if the component has been
unmounted in the meantime.

This is the repro for #9. The test is expected to fail, as I have not attempted to fix the issue. Please let me know if this repro makes the issue I reported in #9 clear or not.

@sheaivey
Copy link
Copy Markdown
Owner

Can you resolve these conflicts then let me know if the latest changes around setState() have fixed this issue in v1.0.3?

The following lines address issue #9
Don't start any new requests after componentWillUnmount()
https://github.com/sheaivey/react-axios/blob/master/src/components/Request.js#L55

Don't set state after async success and error
https://github.com/sheaivey/react-axios/blob/master/src/components/Request.js#L76
https://github.com/sheaivey/react-axios/blob/master/src/components/Request.js#L80

This failing test case reproduces the error I reported in sheaivey#9.

The logic error is that the debounce will call the wrapped function
and the wrapped function does not check if the component has been
unmounted in the meantime.
@binki
binki force-pushed the test-unmounted-setstate branch from 65a5d7a to 8d9bf0f Compare August 1, 2017 01:13
@binki
binki commented Aug 1, 2017
Copy link
Copy Markdown
Contributor Author

@sheaivey I rebased and it looks like it’s passing at least the tests I added for the PR. Thanks!

@sheaivey
sheaivey merged commit 8876472 into sheaivey:master Aug 4, 2017
@sheaivey
sheaivey commented Aug 4, 2017
8000 Copy link
Copy Markdown
Owner

Great, thanks for the contribution!

@binki
binki deleted the test-unmounted-setstate branch August 5, 2017 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0