reset last results on new observer subscribe

[roycclu]

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.

[apollo-cla]

@roycclu: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[apollo-cla]

	Fails
🚫	Running your Dangerfile has Failed

Danger has errored

Error: TypeError

TypeError: Cannot read property 'login' of null
    at Object.<anonymous>.commits.map.x (dangerfile.ts:45:43)
    at Array.map (<anonymous>)
    at Object.<anonymous> (dangerfile.ts:45:25)
    at Runtime._execModule (/home/circleci/project/node_modules/jest-runtime/build/index.js:513:13)
    at Runtime.requireModule (/home/circleci/project/node_modules/jest-runtime/build/index.js:329:14)
    at /home/circleci/project/node_modules/danger/distribution/runner/DangerfileRunner.js:133:33
    at ensureCleanDangerfile (/home/circleci/project/node_modules/danger/distribution/runner/DangerfileRunner.js:173:5)
    at Object.<anonymous> (/home/circleci/project/node_modules/danger/distribution/runner/DangerfileRunner.js:132:21)
    at step (/home/circleci/project/node_modules/danger/distribution/runner/DangerfileRunner.js:32:23)
    at Object.next (/home/circleci/project/node_modules/danger/distribution/runner/DangerfileRunner.js:13:53)

Generated by 🚫 dangerJS

[jbaxleyiii]

@roycclu what is the goal with this?

[roycclu]

@jbaxleyiii solving issue #2774 #2513
It seems without resetting the query for results from network. Queries are getting stale results through the queryRecycler mechanism.

[viggyfresh]

Could be totally mistaken, but I believe this addresses apollographql/react-apollo#1385 as well! Any reason why this hasn't gone in yet? Thanks everyone!

[nfantone]

@jbaxleyiii There are a ton of issues about this both at this repo and react-apollo's.

#2513
apollographql/react-apollo#1389
#2774
apollographql/react-apollo#1229
apollographql/react-apollo#1466
#2669
#2687

I'm sure there are more out there. It's a very critical issue. And people are coming up with quick patches and alternative packages.

I don't know if this is the actual solution to the problem, but I'm respectfully surprised at your question.

[jbaxleyiii]

@nfantone all of those issues should be fixed with the new react apollo. The QueryRecycler was a nightmare that made a ton of issues like this a problem.

As for in Apollo Client, the question to me is should we resetResults when new subscribers add on. I don't think so. A simple case shows where this is a problem:

client.watchQuery({ query )).subscribe((data) => {
  // when we have data, create a new subscription (its like mounting a sub component)
  client.watchQuery({ query }).subscribe((result) => {
    // why would this reset the last results since its just a cache read
  });
});

What I would love to see here for a core change is a failing test showing the behavoir that isn't working currently or a reproduction with the latest updates showing this is still a problem.

I'll work next week to hit those issues (THANK YOU for listing them here) to see if they are fixed in the new releases

I don't know if this is the actual solution to the problem, but I'm respectfully surprised at your question.

Wearing the community member hat, I totally understand this! As the primary maintainer, I'd love some empathy here as our team is incredibly small (but growing) and keeping on top of everything is sooo hard. I really appreciate your feedback and the contributions here and am trying to keep up I promise 👍

[hwillson]

Thanks for contributing this PR @roycclu! As mentioned in #2871 (comment), a lot (maybe all) of the issues listed in this thread should now be fixed, with recent versions of apollo-client and react-apollo. As also mentioned in that comment, there are scenarios where the changes you've submitted in this PR will likely cause problems. If you're able to put together some failing tests or a reproduction that shows this problem, and how your PR helps address it (without impacting the rest of the codebase), then that would help move this PR along. For now though since there hasn't been any traction here in a while, I'll close this off. Thanks again for your work on this!

[sjnonweb]

@hwillson Is this released on NPM, because i'm still facing this issue, These are the latest packages i'm using:

• react-apollo: 2.1.9
• apollo-client: 2.3.7

[gandhiamarnadh]

@sjnonweb these versions fixed for me, just a note make sure you have apollo-client 2.3.7 in node modules

edit: sorry this didn't fix, a error() was thrown un intentionally which was caught by error handler which uses the same error representation component I was mistaken. Surprisingly, error() thrown will achieve the desired behavior graphql errors are reset and the query resulted in error when executed resolves on network again.

[gandhiamarnadh]

@hwillson I also dont see this code on master can you please confirm on which version is this deployed on