-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Move testHook to its own pacakge #302
Comments
I'm all for this https://twitter.com/donavon/status/1096061809499561984
|
You make it, and I'll make this happen |
Yeah 🤸 |
I have some family obligations today, but will get on this as soon as I can. But go out and grab the npm name |
Oh cool! Could you give it a look and open an issue for anything that's lacking/different and see if the maintainer is interested in doing/accepting what we need? |
Well, we also pulled a Webpack and released with no docs. I think this is pretty prominently explained in the docs PR for testHook: testing-library/testing-library-docs#32 The linked package seems pretty close in API - I'll file an issue since you both are indisposed. |
Hey, author of I started out with a "what would As I said in the other issue, it started out as a proof-of-concept from my perspective and I only updated it recently to match the stable hooks API in case anyone was actually using it. I've had a few thoughts on how the API could be improved, but nothing nailed down. I'm more than happy to accept PRs bringing it inline with any features offered by |
Just wanted to let you all know that I have published a |
Awesome. I think we're ready to make this happen. Anyone want to make the PR too remove testHook? |
I can handle this. :) |
This removes testHook in favor of http://npm.im/react-hook-testing-library Closes #302 BREAKING CHANGE: move testHook to another library. Use http://npm.im/react-hook-testing-library instead
🎉 This issue has been resolved in version 6.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Oh wow, that was fast. Just a couple of days ago I was contributing to Can someone please enlighten me why it happened? Yes, I see the tweet and I still don't get it :) When someone puts an effort into separating logic into a custom hook, I don't see why it should be bad being able to test it. It has a name and a clear purpose. I am kinda sad there wasn't more discussion about this. |
You may think that its purpose is clear, but in the few days it existed we already saw people mistreating it. Custom hooks are implementation details 95% of the time. It's not all that hard for library authors to install |
I know it's not hard. And that's why I am wondering how will that stop the actual "mistreat" of hooks? :) Feels like you have just dropped a slight detour on the road, but the destination is still the same ;) |
Trust me, the detour is enough. |
Sure, trust is a nice thing :) But for me, I am going to install that detour to the app (not library) and just use it because it's helpful. I don't think it's correct to say, that all hooks are an implementation detail. I have plenty of hooks that are reusable just within the app code. |
not foundpackage 'react-hook-testing-library' not found |
@smmoosavi It was just a missing 's'. |
There is the same mistake in the release page |
Btw, how will library authors find out about that package? Is it going to be in the docs? I don't expect people studying release notes :) If that's the case, how is it going to prevent claimed misuse by other people? If it's not going to be mentioned anywhere, library authors will just stitch together some custom solution so it's all for nothing. I don't know, it's a strangely controversial decision. Instead of teaching people how to do things properly we try to hide things from them and hope for the best. No, I don't see it, really. |
@FredyC I'd hope we could move these WIP docs and examples onto an ecosystem page like user-event and jest-dom: testing-library/testing-library-docs#32 cc @mpeyper |
@alexkrolick Ok sure, but I am still kinda missing a whole reason why this was done. If it's going to be documented in official docs, how is that going to prevent anyone from abusing it? The focus should be really on teaching correct patterns and not moving things around just to make it interesting 😄 @kentcdodds It's not like it's going to get reverted, I get that. I just kinda want to understand how hiding, but not really hiding things can achieve smarter tests 💭 |
It's all got to do with inertia. when doing the wrong thing is really easy then more people do the wrong thing. But when it takes even a little bit extra effort to do the wrong thing then the only people willing to put forth the effort are the people who actually are doing it because it's the right thing to do. |
I see, thanks. Although in this case, it feels like that much bigger inertia comes with testing hooks without any utility than installing an extra package 😎 |
@donavon When you get a chance, you should update your blog post Intro to React Hooks to reflect this move. |
Is it possible to move |
There's an open issue on that repo to do this. I want it 👍 |
@kentcdodds I'm all for moving How do we make this happen? I'm assuming this means moving the repo from |
Hey @mpeyper! That's great! I'll get you all the permissions set up. You'll need to accept the invitations which will be sent to you shortly. If it's helpful, you can watch me do this for react,dom,cypress-testing-library on the video linked here: testing-library/dom-testing-library#260 (comment) Thanks! |
I thought this issue existed already, but it didn't. I apologize for not send you this earlier. Good luck! testing-library/react-hooks-testing-library#99 |
It seems to be causing a lot of confusion for people and we definitely don't want application developers using it to "unit test" their hooks. Putting it into another package would make it easier for us to call out its true purpose and reduce the chance of misuse.
Thoughts?
The text was updated successfully, but these errors were encountered: