-
Notifications
You must be signed in to change notification settings - Fork 115
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
Improve docs and tests for useMedia hook #150
Comments
Hi @edorivai you can check the third item now |
Hey, I could work on tests for the hook. Is there anything specific that I should know of before starting? |
@0ctothorp let me push a branch that I've been working on. Transliterating the existing tests shouldn't be difficult, but the thing that I was finding hard is that testing the SSR behavior. |
Oh, I didn't know it was being worked on already 😅 |
I've stalled out :P I would very much appreciate the help if you're willing and able. The big thing is that for testing SSR behavior, you want to watch three states and make sure that they're all consistent with each other:
In a two-pass render, you want the first two to agree, and then for the change to happen between the second and third. I'm having trouble testing this, though, because the React Testing Library doesn't really account for a desire to see the middle state and be able to compare it to the first and third states. I was working on a workaround of some sort for that when I stalled out. |
Here's a link to the branch I've been working on: And here's a link to the diff between it and current master: It's very much a work in progress - I started writing the hydration tests by copying and pasting a bunch of existing tests, then went and started trying to solve my problem above, and I didn't get much further than that. Tests are "passing," but that's because they're not actually capturing the desired behavior. |
Cool, I'm going to have a look. I'm definitely not an expert in such tests, but maybe I will be able to come up with something :) |
Awesome. Thank you so much! |
@tstirrat15 implemented the
useMedia()
hook in #149 🎉Two minor improvements that are left open:
index.d.ts
Any help is appreciated! 🙏
The text was updated successfully, but these errors were encountered: