search

apollographql / apollo-link

:link: Interface for fetching and modifying control flow of GraphQL requests
https://www.apollographql.com/docs/link/
MIT License
1.44k stars 344 forks source link

Support subscriptions in apollo-link-schema #917

Open fubhy opened 5 years ago

fubhy commented 5 years ago

Expected Behavior Subscriptions should be supported by SchemaLink.

Actual Behavior SchemaLink currently does not support subscriptions.

https://github.com/apollographql/apollo-link/pull/916

fubhy commented 5 years ago

This repository probably gives a bit more context as to why / where this would be useful: https://github.com/fubhy/graphql-transport-electron

fubhy commented 5 years ago

Bump! This is a really minor change with a lot of value imho :-)

JoviDeCroock commented 5 years ago

The PR has conflicts and is not passing through the CI because it is stale, I'll try to get this discussed asap. Sorry for the wait.

An additional argument to passing on on the PR could be the bundle size that is being added, 2 new modules is quite significant while all the recent effort has gone towards reducing size.

fubhy commented 5 years ago

The PR has conflicts and is not passing through the CI because it is stale, I'll try to get this discussed asap. Sorry for the wait.

An additional argument to passing on on the PR could be the bundle size that is being added, 2 new modules is quite significant while all the recent effort has gone towards reducing size.

Thanks for the reply.

apollo-link-schema is a server side dependency in the vast majority of cases I'd argue. If you are using apollo-link-schema in the browser then you are also running a graphql server in the browser (at which point a tiny size increase of <1kb is not going to be an issue. Iterall has 1.000.000 weekly downloads on npm and is widely used. So it's likely that it's already in your dependenices anywas. And apollo-utilities is also part of the apollo stack and it's also probably installed on the vast majority of graphql servers.

Bottomline: These 2 dependencies very very likely don't add anything.

I fixed the merge conflict in package.json

fubhy commented 5 years ago

@hwillson Hey there, can you take another look and reconsider this to be merged?

definitelycarter commented 5 years ago

Hey @hwillson / @fubhy - I too am looking for this. Has there been any movement? What are the blockers?

fubhy commented 5 years ago

From my PoV there is nothing stopping this from getting merged. I have been using this sucessfully already but since I didn't get any more replies from the maintainers here I added my own independent implementation to my package here: https://github.com/fubhy/graphql-transport-electron

hourliert commented 5 years ago

Any news on this one? It feels that this would be a very valuable improvement. 👍

kolychev commented 5 years ago

We are using apollo-link-schema in our integration tests. It allows not to start graphql server, keep each test run time lower. Would be great to have this PR merged so we can test our subscriptions code with apollo-link-schema. And yes, in our case we don't care about bundle size because it is used only in tests.