Skip to content
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

Open
2 of 3 tasks
edorivai opened this issue May 27, 2020 · 8 comments
Open
2 of 3 tasks

Improve docs and tests for useMedia hook #150

edorivai opened this issue May 27, 2020 · 8 comments

Comments

@edorivai
Copy link
Collaborator

edorivai commented May 27, 2020

@tstirrat15 implemented the useMedia() hook in #149 🎉

Two minor improvements that are left open:

  • Add tests for the hook
  • Add documentation on how to use the hook
  • Add types to index.d.ts

Any help is appreciated! 🙏

@abenhamdine
Copy link
Contributor

Hi @edorivai you can check the third item now

@0ctothorp
Copy link

Hey, I could work on tests for the hook. Is there anything specific that I should know of before starting?

@tstirrat15
Copy link
Contributor

@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.

@0ctothorp
Copy link

Oh, I didn't know it was being worked on already 😅

@tstirrat15
Copy link
Contributor

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:

  1. the server-rendered markup
  2. the first hydration pass
  3. the first full render pass

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.

@tstirrat15
Copy link
Contributor

Here's a link to the branch I've been working on:

https://github.com/tstirrat15/react-media/tree/155-testing-and-implementing-two-pass-render

And here's a link to the diff between it and current master:

https://github.com/ReactTraining/react-media/compare/master...tstirrat15:155-testing-and-implementing-two-pass-render?expand=1

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.

@0ctothorp
Copy link

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 :)

@tstirrat15
Copy link
Contributor

Awesome. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants