Skip to content

Conversation

@jackdclark
Copy link
Contributor

@jackdclark jackdclark commented Feb 19, 2019

Part of #9

Add test coverage for useQuery

 PASS  test/unit/useQuery.test.js
  useQuery
    ✓ calls useClientRequest with query (22ms)
    ✓ calls useClientRequest with options (2ms)
    ✓ returns initial state from useClientRequest & refetch (2ms)
    ✓ bypasses cache when refetch is called (1ms)
    ✓ sends the query on mount if no data & no error (2ms)
    ✓ does not send the same query on mount if data is already present (2ms)
    ✓ does not send the query on mount if there is an error (1ms)
    ✓ adds query to ssrPromises when in ssrMode if no data & no error (3ms)
    ✓ does not add query to ssrPromises when in ssrMode if there is already data (1ms)
    ✓ does not add query to ssrPromises when in ssrMode if there is an error (1ms)
    ✓ does not add query to ssrPromises when in ssrMode if ssr is overridden in options (2ms)
    ✓ does not send the same query twice (2ms)
    ✓ sends the query again if the variables change (5ms)
    ✓ sends another query if the query changes (2ms)

@jackdclark jackdclark requested a review from bmullan91 February 19, 2019 09:40
@jackdclark jackdclark requested a review from Joezo February 19, 2019 10:02
Joezo
Joezo previously approved these changes Feb 19, 2019
Copy link
Contributor

@Joezo Joezo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests look good 👍

Copy link
Contributor

@bmullan91 bmullan91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good - a couple of things missing:

  1. Testing the refetch function re-calls fetchData passing skipCache=true Sorry, you do have that test, my bad.
  2. Is it possible to verify the second argument to the useEffect [query, JSON.stringify(opts.variables)] i.e. change the variables causes it to be re-called

@jackdclark
Copy link
Contributor Author

@Joezo @bmullan91 thanks for the reviews. I've added the following three tests to cover the useEffect conditional firing

    ✓ does not send the same query twice (2ms)
    ✓ sends the query again if the variables change (2ms)
    ✓ sends another query if the query changes (2ms)

Copy link
Contributor

@Joezo Joezo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks even better now 👍 🥇

Copy link
Contributor

@bmullan91 bmullan91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - nice work @jackdclark 😎

@jackdclark jackdclark merged commit 654ef22 into master Feb 19, 2019
@jackdclark jackdclark deleted the test-use-query branch February 19, 2019 13:04
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.

3 participants