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

Use standby fetchPolicy for recycled queries #671

Merged
merged 7 commits into from May 3, 2017

Conversation

Projects
None yet
3 participants
@jacobk
Copy link
Contributor

jacobk commented May 1, 2017

See #622

TODO:

  • Add failing test case (test passes with #531 reverted)
  • Use standby fetchPolicy when apollographql/apollo-client#1636 is merged
  • Update test case with more assertions
  • Update to apollo-client 1.1.2
});
break;
case 2:
// // TODO: Proper assertions

This comment has been minimized.

@jacobk

jacobk May 1, 2017

Author Contributor

@helfer I noticed that the recycled component is rendered twice after being recycled and rendered again. Is this to be expected?

This comment has been minimized.

@helfer

helfer May 1, 2017

Member

It depends. Maybe it renders twice because it tries to do a network fetch? Or does it render twice with the same parameters?

This comment has been minimized.

@jacobk

jacobk May 1, 2017

Author Contributor

Props are visually identical but apollo data-functions are changed due to observableQueryFields that seems to be invoked a couple of times between renders. I haven't studied that in detail but seems unrelated to this test-case.

Jacob Kristhammar
@jacobk

This comment has been minimized.

Copy link
Contributor Author

jacobk commented May 1, 2017

I'm pretty happy with the current test case. Should be an easy fix once standby lands in apollo-client.

@jacobk

This comment has been minimized.

Copy link
Contributor Author

jacobk commented May 3, 2017

@helfer I noticed that apollo-client with standby is released. I've updated this PR to use standby and tried it against apollo-client 1.1.2 and all tests seems to pass.

Should be ready to merge once rebased after react-apollo upgrades apollo-client.

EDIT

Hmm, also seems to pass above, the TS shouldn't compile?

EDIT2

Ah, apollo-client is a caret range dep so it picks up the new apollo-client 👍

@jacobk jacobk changed the title WIP: Use standby fetchPolicy for recycled queries Use standby fetchPolicy for recycled queries May 3, 2017

Jacob Kristhammar
@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented May 3, 2017

@jacobk can you rebase this, and update the changelog and package.json file to force ^1.1.2 of apollo-client? I'd love to merge this in and release it!

Thank you for the great tests!

jacobk and others added some commits May 3, 2017

@jacobk

This comment has been minimized.

Copy link
Contributor Author

jacobk commented May 3, 2017

@jbaxleyiii no problem! Should be done now.

@helfer

This comment has been minimized.

Copy link
Member

helfer commented May 3, 2017

Nice, I'm looking forward to seeing this released in the wild @jacobk @jbaxleyiii !

@jbaxleyiii jbaxleyiii merged commit e12fbb1 into apollographql:master May 3, 2017

3 checks passed

CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 92.255%
Details
@jbaxleyiii

This comment has been minimized.

Copy link
Member

jbaxleyiii commented May 3, 2017

@helfer @jacobk same! I'll release tonight!

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