Fix query result when skip=true (#1869)

[edorivai]

Should fix #1869

I added a test case, and took the implementation from the original PR that aimed to implement the skip prop on <Query>.

I suspect this piece of logic got lost in the big rewrite mentioned here.

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

[edorivai]

Not sure what's up with that failing test. Is it because of the changes in this PR?

[ZwaarContrast]

Any progress on this?

[caseywebdev]

Ran into this issue yesterday, can a maintainer chime in here? I really like the Query component but I also need the skip and loading values to work correctly 😉

[k00k]

@edorivai care to merge master back in? Any idea what we need to do to get this PR merged?

[edorivai]

@k00k I've merged back in, seems like all checks passed. As far as I know this PR should be good to go.

[k00k]

All checks pass. Looks clean.

[hwillson]

Quick note here - I've re-arranged this a bit to short circuit un-necessary extra query logic, when the query is to be skipped. One thing to add though, is that this means we're leaving networkStatus as undefined. This is currently intentional, as setting this to NetworkStatus.ready can cause problems when using something like the graphql hoc and calculating the skip value using a function (forcing a NetworkStatus.ready value can prematurely end rendering). Leaving networkStatus as undefined here seems to be okay though, and is better than the way it was before when we did the skip check at the end of getQueryResult. That left networkStatus in NetworkStatus.loading state, even though we weren't loading anything.

[caseywebdev]

What's the status on this one? Seems like it's good to go (besides conflicts).

[miafoo]

We've run into this issue as well and would really appreciate it being merged in. 😃

[rosskevin]

Please resolve conflicts, otherwise I think this good to merge

[edorivai]

@hwillson are you picking up the conflicts, or would you like me to do it?

[hwillson]

@edorivai I'll take care of it shortly. Thanks!

[stephane-ruhlmann]

Is anyone taking care of this?

[Kjagd]

@hwillson What's the status on this? We've been greatly affected by this for quite some time now.

[pascalstr]

@hwillson what is the status of resolving these conflicts and merging this PR?

[hwillson]

We're good to go here @edorivai - sorry for the long delay, and thanks for working on this!

[edorivai]

Great to hear 🎉

[sbogdaniuk]

SO you just fixed this issue for Component, and for HOC not, right?