Apollo-client: subscribeToMore bad documentations and typescript type annotations

[JozefBiros]

Intended outcome:
According to new docs about subscriptions:
https://www.apollographql.com/docs/react/features/subscriptions.html#subscribe-to-more

Data about new comment should be found in
subscriptionData.data.commentAdded

Typescript annotation for subscriptionData is
subscriptionData: { data: any }

Actual outcome:
But data about new comment are direcetly in subscriptionData like this:
subscriptionData.commentAdded

Typescript annotation for subscriptionData should be according to data something like
subscriptionData: any

How to reproduce the issue:
Try subscription example from docs.

Version

• apollo-client@<2.0.1>
• react-apollo@<2.0.0>

[jbaxleyiii]

@JozefBiros hmm it seems like we went back and forth on this! I'll ping a few subscription contributors to see what it should ideally be. I lean towards what is documented because it gives us the ability to add errors as well, but I think the previous version did it differently?

@mistic @Urigo @DxCx @NeoPhi what is the right direction here?

[DxCx]

and which subscription transport?
using transport >= 0.9.0 should yield subscriptionData: { data: any }
post that will yield subscriptionData: any

[dnalborczyk]

we just upgraded our project, and were wondering the same if removing the _data_ property was intended?

@DxCx as far as I can tell it doesn't seem to matter which version of subscription transport one is using, it's transferring { _data_ } either way. (< 0.9 and > 0.9).

the _data_ property is being removed by apollo-client v2.0

https://github.com/apollographql/apollo-client/blob/apollo-client%402.0.0/packages/apollo-client/src/core/QueryManager.ts#L889

compare to v1.9:

https://github.com/apollographql/apollo-client/blob/v1.9.0/src/core/QueryManager.ts#L922

as @JozefBiros indicates, the docs seem to contradict.

[dnalborczyk]

related issue:
https://github.com/apollographql/apollo-client/issues/2329(https://github.com/apollographql/apollo-client/issues/2329)

related commit:
https://github.com/apollographql/apollo-client/commit/f8990960468a13e87e514c1a2b985e03069fa9ab

[DxCx]

ok so 2 things happened here:
subscription transport added data when introducing observable interface (as it should be).

after that james worked on subscription on apollo client 2
and returned the old behavior.. probably to stay compatible with older versions.

[dnalborczyk]

@DxCx I'm not quite sure if I'm following. I'm mainly wondering if the above change was intended, which is fine with me, other than the docs seem to be wrong - or otherwise it might need to be fixed before a broader adoption of v2.0.

[cesarsolorzano]

If returning the data property directly is intentional, it should be better documented. Everyone using subscribeToMore will have to change their code when they migrate to Apollo 2.0.

UPDATE:
Also, if it's intended, it's necessary to update GitHunt

https://github.com/apollographql/GitHunt-React/blob/cb225f4852518a912e8cc6355e777e6d65fd97d1/src/routes/CommentsPage.js#L48

[jbaxleyiii]

Looking at the olds docs (https://s3.amazonaws.com/apollo-docs-1.x/subscriptions.html#subscribe-to-more) subscribeToMore should return { data, errors }. Therefore I think this was a bug that I introduced in the 2.0 and should be fixed as such!

If anyone wants to open a PR I'd be thrilled to review and merge, otherwise I will add it to my list for this week.

Thanks to @dnalborczyk , @DxCx and @cesarsolorzano for chiming in here and to @JozefBiros for opening this issue!