Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Stale variables when combining pollInterval, skip, forceFetch #374

Closed
jbinto opened this issue Dec 20, 2016 · 5 comments
Closed

Stale variables when combining pollInterval, skip, forceFetch #374

jbinto opened this issue Dec 20, 2016 · 5 comments
Assignees
Labels

Comments

@jbinto
Copy link

jbinto commented Dec 20, 2016

This seems similar to #358 (but possibly unrelated).

Here is my situation:

  • When I click a button, I issue a mutation. I get an ID back (e.g. id=1).
  • A polling query starts (since the value of a skip became false). options is called with ownProps, and that has the ID from the mutation (e.g. id=1).
  • Polling begins, and network inspector shows the query is using the correct ID (e.g. id=1)
  • After the polling query sees changes in some state, we flip skip again and polling stops.
  • All is well at this point.

But, if I click the button again, this happens:

  • The mutation is issued. I get a new ID back (e.g. id=2)
  • A polling query starts (since the value of a skip became false). options is called with ownProps, and that has the ID from the mutation (e.g. id=2).
  • Polling begins, and network inspector shows the query is using the WRONG ID, from the previous variables. (e.g. id=1)
  • This breaks other things in my app because it was not expecting a result for id=1 but for id=2.

Steps to Reproduce

I've isolated this in a reproduction case:

git clone https://github.com/jbinto/apollo_polling_bug/
cd apollo_polling_bug
yarn
npm start

To keep things simple I don't do a mutation, just controlling props with setTimeout is sufficient to reproduce. I used a mock network interface that just echoes back an argument.

Relevant files:

https://github.com/jbinto/apollo_polling_bug/blob/master/src/index.js
https://github.com/jbinto/apollo_polling_bug/blob/master/src/Foo.js

Buggy Behavior

Somehow, apollo-client or react-apollo is holding on to old variables and issuing queries based on those, despite the fact the props of the graphql HOC have changed.

Expected Behavior

I should be able to combine skip, forceFetch, and pollInterval as such:

  • forceFetch and pollInterval are fixed as true, and 1000 respectively. skip begins as true.
  • Set skip to false, by re-rendering with new props (including an id prop, which is used in options.variables)
  • Let it poll for a bit
  • Set skip to true, polling stops
  • Let it idle for a bit
  • Set skip to false, and send a new id prop
  • Polling starts again, but...
  • It should use the new id prop I provided to options.variables, not the old one

Version

  • apollo-client@0.5.23
  • react-apollo@0.7.1
@tmeasday
Copy link
Contributor

Thank you @jbinto for the excellent reproduction.

Can confirm the problem and also that it's not AC (see this branch: https://github.com/tmeasday/apollo_polling_bug/tree/pure-apollo-client)

@tmeasday tmeasday added the bug label Dec 21, 2016
@tmeasday tmeasday self-assigned this Dec 21, 2016
@tmeasday
Copy link
Contributor

Ok, so I think the issue is here: https://github.com/apollostack/react-apollo/blob/master/src/graphql.tsx#L343-L345

It assumes that we've either (a) just created the query with the current options and need to create a subscription or (b) we may be changing the options and the subscription should exist.

This is obviously third case that needs to be dealt with.

@tmeasday
Copy link
Contributor

Started working on a solution, hit a couple of snags, will try and resolve the snags soon unless someone else solves them for me: #376

@tmeasday
Copy link
Contributor

(Also this is quite annoying: #375)

@jbinto
Copy link
Author

jbinto commented Dec 22, 2016

Thanks for the quick turnaround!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants