Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

observer is not defined #30

Closed
kachkaev opened this issue Nov 3, 2017 · 3 comments
Closed

observer is not defined #30

kachkaev opened this issue Nov 3, 2017 · 3 comments

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Nov 3, 2017 •
edited
Loading

Hi Oleg,

Looks like the subscriptions are not working in the latest version. GraphiQLSubscriptionsFetcher.js refers to a variable called observer, but it is not defined. Getting an exception when trying to subscribe to something.

screenshot from 2017-11-03 16-52-54

I'm also wondering if sending subscriptions could be disabled when the ws url is not defined. I forgot to define it today and sent a subscription, but this resulted an odd error, because the query was sent to the HTTP server:

screenshot from 2017-11-03 16-54-30

Apollo tried resolving a subscription when there's no event, so the payload was undefined.
@tlvenn
Copy link
Contributor

tlvenn commented Nov 9, 2017

Yep @OlegIlyenko broke it with this commit: 6258979
The dependency on subscription-transport-ws should target 0.8.x, they have deprecated the API the fetcher is using in 0.9.x

I will submit a PR shortly to fix this.

tlvenn added a commit to tlvenn/graphiql-workspace that referenced this issue Nov 9, 2017
@tlvenn
Update subscription-transport-ws version
70618dd 
The current api we use in the fetcher expects the 0.8.x version of  subscription-transport-ws. There are breaking changes in the 0.9.x

I have also taken the opportunity to use yarn selective dependency resolution to better address the graphql issue without resorting to edit the yarn.lock file.

Fixes OlegIlyenko#30
@tlvenn tlvenn mentioned this issue Nov 9, 2017
Merged
@OlegIlyenko
Copy link
Owner

Just published v1.1.4 Sorry for breaking the deps 😰 Thanks again got fixing it, @tlvenn!

@kachkaev
Copy link
Contributor Author

Thanks Oleg! Updating to 1.4.4 worked for me in my create-react-app instance, although I had to switch to yarn in order to avoid a duplicate graphql dependency (versions 0.11.* and 0.10.*). @tlvenn many thanks for introducing the "resolutions" section in package.json – otherwise I'd fiddle with duplicate deps forever 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@OlegIlyenko @kachkaev @tlvenn