Skip to content

Add getInstance helper #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

benadamstyles
Copy link
Contributor

This allows testing instance.state and other advanced patterns using the component instance.

This allows testing `instance.state` and other advanced patterns using the component instance.
@benadamstyles
Copy link
Contributor Author

(Obviously I'm happy to writes tests and types, I just wanted to get feedback on the idea first!)

@thymikee
Copy link
Member

thymikee commented Oct 20, 2018

Sorry to be a bearer of "bad" news, but this is a design decision behind this library not to export getInstance. Testing internal component instance is basically testing implementation details (e.g. component will change to function from class and the test will fail). If you have parts of the state management that you'd like to test e.g. for edge cases, export that piece of functionality and test in isolation.

I'm thankful for the PR though, that's great attitude!

@thymikee thymikee closed this Oct 20, 2018
@benadamstyles
Copy link
Contributor Author

Fair enough, no problem! Maybe you should change the docs here which state:

For convenience it also returns react-test-renderer's instance and renderer objects, in case you need more control.

That actually isn't the case, but it made me think that you might be open to adding that functionality back in.

@benadamstyles benadamstyles deleted the patch-1 branch October 20, 2018 14:07
@thymikee
Copy link
Member

Ahh forgot to remove that! Would you like to send a PR?

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