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

Add test for QueryManager continuing to poll after refetch (see issue #239) #242

Merged
merged 3 commits into from
Jun 11, 2016
Merged

Conversation

ciwolsey
Copy link
Contributor

Test regarding issue #239. Hope this is okay.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.6%) to 93.581% when pulling c1f9dc2 on ciwolsey:master into df0edf6 on apollostack:master.

@stubailo
Copy link
Contributor

Thanks very much!

@abhiaiyer91
Copy link
Contributor

Im down to solve this one if @ciwolsey doesn't want to

@ciwolsey
Copy link
Contributor Author

Added a fix that greens the test too but I'm definitely not sure I've done it in the best way.

@@ -220,13 +220,6 @@ export class QueryManager {
this.stopQuery(queryId);
},
refetch: (variables: any): Promise<GraphQLResult> => {
// if we are refetching, we clear out the polling interval
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem we're trying to fix here is that calling refetch currently kills polling completely, but that's not the intended outcome - ideally the client will keep polling after the forced refetch.

@helfer
Copy link
Contributor

helfer commented May 31, 2016

@ciwolsey I think this does what we need, but the linter checks are not passing. Can you look into that?

@helfer
Copy link
Contributor

helfer commented Jun 4, 2016

@ciwolsey Any update on this? It seems the only thing that you'd have to fix is this:

test/QueryManager.ts[753, 9]: if statements must be braced

@ciwolsey
Copy link
Contributor Author

ciwolsey commented Jun 10, 2016

@helfer Fixed the if statement

@helfer
Copy link
Contributor

helfer commented Jun 10, 2016

@ciwolsey Thanks a lot! Could you do a rebase as well? We've merged some other PRs into master since you made this one.

@helfer helfer merged commit 4cbd826 into apollographql:master Jun 11, 2016
@helfer
Copy link
Contributor

helfer commented Jun 11, 2016

Merged! Thanks a lot!

jbaxleyiii pushed a commit that referenced this pull request Oct 17, 2017
Clarifies the Apollo Meteor docs
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants