Fixes to some infinite rendering loop bugs

[brainkim]

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

[brainkim]

Fixes #7139, #7313, #7493 (Maybe, difficult to reproduce), #7780

Additional changes we can make for better DX:

• Warn when HttpLink encounters a subscription query.

---

Deep dive into why this was happening for posterity. If you’re not reading through the apollo-client codebase source, then this might not make sense to you.

Multiple useSubscription calls were triggering calls to SubscriptionData.startSubscription per render loop, but SubscriptionData.endSubscription was being called on only one of the internal subscription objects at the end of the render under certain conditions. This caused a ping-pong effect where one of the useSubscription calls is reinitialized each render. This behavior is exacerbated by Concast, which upon reinitialization, will respond with last message and trigger another render.

Call order (Happy path with 1 useSubscription call, Sad path with 2 useSubscription calls, Fix with 2 useSubscription calls)

• HttpLink, 1 call to useSubscription
  • useSubscription initializes
  • SubscriptionData constructor
  • subscriptionData.endSubscription is called (initial)
  • Concast is created
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist)
  • concast subscriber is called
  • concast.deliverLastMessage is called (this.latest is not set)
  • HttpLink fetch call fulfills (observer.next and observer.complete is called)
  • subscriptionData.updateResult is called (triggers setState in the React world)
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists)
  • subscriptionData.endSubscription is called via completeSubscription
  • Quiescence 🎉
• HttpLink, 2 calls to useSubscription
  • useSubscription initializes 1
  • SubscriptionData constructor 1
  • subscriptionData.endSubscription is called (initial) 1
  • Concast is created 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist) 1
  • concast subscriber is called 1
  • concast.deliverLastMessage is called (this.latest is not set) 1
  • useSubscription initializes 2
  • SubscriptionData constructor 2
  • subscriptionData.endSubscription is called (initial) 2
  • Concast is created 2
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist) 2
  • concast subscriber is called 2
  • concast.deliverLastMessage is called (this.latest is not set) 2
  • HttpLink fetch call fulfills 1
  • subscriptionData.updateResult is called 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 2
  • subscriptionData.endSubscription is called 1
  • HttpLink fetch call fulfills 2
  • !!! INFINITE LOOP START !!!
  • subscriptionData.updateResult is called 2
  • subscriptionData.startSubscription is called (this.currentObserable.subscription does not exist) 1
  • concast subscriber is called 1
  • concast.deliverLastMessage is called (this.latest is set) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 2
  • subscriptionData.endSubscription is called 2
  • subscriptionData.updateResult is called 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist) 2
  • concast subscriber is called 2
  • concast.deliverLastMessage is called (this.latest is set) 2
  • subscriptionData.endSubscription is called 1
  • !!! GO TO INFINITE LOOP START !!!
  • Repeat ad infinitum, QED 😰
• Fixed
  • useSubscription initializes 1
  • SubscriptionData constructor 1
  • subscriptionData.endSubscription is called (initial) 1
  • Concast is created 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist) 1
  • concast subscriber is called 1
  • concast.deliverLastMessage is called (this.latest is not set) 1
  • useSubscription initializes 2
  • SubscriptionData constructor 2
  • subscriptionData.endSubscription is called (initial) 2
  • Concast is created 2
  • subscriptionData.startSubscription is called (this.currentObservable.subscription does not exist) 2
  • concast subscriber is called 2
  • concast.deliverLastMessage is called (this.latest is not set) 2
  • HttpLink fetch call fulfills 1
  • subscriptionData.updateResult is called 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 2
  • subscriptionData.endSubscription is called 1
  • HttpLink fetch call fulfills 2
  • subscriptionData.updateResult is called 2
  • subscriptionData.startSubscription is called (this.currentObserable.subscription does not exist) 1
  • concast subscriber is called 1
  • concast.deliverLastMessage is called (this.latest is set) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 2
  • subscriptionData.updateResult is called 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 1
  • subscriptionData.startSubscription is called (this.currentObservable.subscription exists) 2
  • subscriptionData.endSubscription is called 2 (THIS STEP HAS MOVED FROM ABOVE)
  • subscriptionData.endSubscription is called 1 (THIS STEP HAS BEEN ADDED)
  • Quiescence 🎉

[brainkim]

I’m happy to make HttpLink warn on subscription queries, just let me know if you want that in this PR.

[benjamn]

LGTM. Can you also update CHANGELOG.md in the style of the other entries (under Bug fixes for 3.3.14) before merging?

I’m happy to make HttpLink warn on subscription queries, just let me know if you want that in this PR.

@brainkim Yep, let's do that in a follow-up PR. I'd like to get this PR into the next patch release (unless you object).

[jcreighton]

Great work!