When multiple parts of the UI utilize the same subscription it will be subscribed to for each occurance.
This increases latency to the user because it doesn't pull from the cache and increases network traffic due to duplicated payloads.
There already appears to be a start at deduplicating in QueryManager, it utilizes an observables array despite it always containing 0 or 1 observable.
For example, I have a <Subscription subscription={ON_NEW_CHAT}> in NewMessageNotification.js and a subscribeToMore({ document: ON_NEW_CHAT }) in Channel.js. I see in the websocket traffic that this creates two identical subscriptions.
For the record, the only thing currently stopping this from working is that we explicitly disable deduplication for subscriptions by passing false for the deduplication parameter of getObservableFromLink here.
Is there any reason it can't be true?
@hwillson Any thoughts on deduplicating subscriptions by default?
@benjamn I'm all for it (and we should be safe to deduplicate subscriptions in a minor/patch release).
Any word on this release? What's the current best practice to have multiple components received subscription events?
@qeternity I believe that ppl are currently moving subscriptions from child components to common parents. It sort of works, but is far from ideal since you end up having complex detached logic in parent components, just to update queries used in child components.
It would be much more pleasant to handle those updates directly in child components, letting us reuse same subscriptions multiple times which should be deduplicated by Apollo automatically.
If it's just a matter of setting an internal flag, would be great to have this enabled.
I believe this would improve traffic for a lot of apps using Apollo subscriptions, since not everyone took the road of abstracting subscriptions to higher levels.
@hwillson @benjamn @jpikora
I am composing useSubscription into a custom react hook. For each call position where the hook is used, a new subscription is created. This deduplication issue is compounded with the addition of @apollo/react-hooks.
Any movement on setting this to true? Would really help performance!
QueryManager has evolved quite a bit. I do now see a constructor parameter, though I am not sure what it takes to set it. https://github.com/apollographql/apollo-client/blob/8c1d8b4dc5ac964dd791bf635918309aa1474b70/src/core/QueryManager.ts#L88
Personally I'm not in a position to dive too deep into this at the moment since client 3 is not playing nice with our app so we are trying to mitigate that first. The GraphQL spec is so bloody complex, I'm tempted again to move to making a graphql-lite in yaml without aliases or type enforcement.
The PR #6910 that enables this is ready and waiting for approval / comments since quite a while, any reason this hasn't been merged in.
Most helpful comment
@benjamn I'm all for it (and we should be safe to deduplicate subscriptions in a minor/patch release).