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.

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

Cannot call map on the observable returned by watchQuery #3721

Closed
fullykubed opened this issue Jul 22, 2018 · 9 comments
Closed

Cannot call map on the observable returned by watchQuery #3721

fullykubed opened this issue Jul 22, 2018 · 9 comments
Labels
🚧 in-triage Issue currently being triaged

Comments

@fullykubed
Copy link

See old issue #2788

Same exact issue reappears in Apollo 2.3.5. Possible regression?

Example code:

        this.infoQuery = client.watchQuery({
            query,
            variables: {userID},
        });
        this.infoQuery
            .map((res) => {
                console.log("res", res);
                return res;
            })
            .subscribe({
                next: (x) => this.setState({}),
            });

Throws
Cannot read property 'variables' of undefined

@joshribakoff
Copy link
Contributor

joshribakoff commented Jul 23, 2018

Same issue when I tried to call reduce() in my PR #3713

My workaround for now is to wrap it with rx and use map/scan operators from rxjs instead

import { Observable } from 'rxjs';

      Observable.create(observer => {
        client.watchQuery({ query }).subscribe({
          next: observer.next,
          error: observer.error,
          complete: observer.complete
        })
      })

@fullykubed
Copy link
Author

That's a good idea. Though it'd be better to not have two observable dependencies in the mix. I'll have to look into this more and Ill report back.

@joshribakoff
Copy link
Contributor

Agreed. Rxjs and zen observable are currently both in package.json and I wasn't sure why. Zen is objectively more lightweight than rxjs. But rxjs is objectively more robust.

No matter what observable implementation we use, a user may want a different implementation and in that case would need to convert the observable by wrapping it with their library of choice

Other libraries like redux observable and cycle js also introduced the concept of adapters to switch between observable implementations I think.
In this instance I think we just have a bug. Somewhere in the process of extending zen observable, some internal state may have got "messed up"... I think it should be possible to fix without major changes to the library, although it maybe a good time to think about long term and which observable implementation Apollo is going to support.

If Apollo is going to only suppprort zen observable, that is fine, but it is worth acknowledging that users of the library may need to wrap the zen observable in an rxjs observable in order to gain access to additional operators

I also think Evan Houser has a PR open to convert Apollo link to rxjs, FYI.

I agree having to convert observables is not ideal. Ideally it should "just work" without having to resort to such a workaround. I suppose how we achieve that maybe up for discussion but I would vote for just switching the library to rxjs, with rxjs 6 it is actually very lightweight if you don't import all the operators. Reviewing the git log I see a lot of code churn related to zen observable. I'm not sure what the right answer is myself either but I would lean towards switching to rxjs since libraries like angular have adopted it and it is backed by Netflix.

@fullykubed
Copy link
Author

I would love rxjs personally. I'm just going to avoid mapping for now. I am running into more pressing bugs with cache redirects.

@joshribakoff
Copy link
Contributor

joshribakoff commented Jul 25, 2018 via email

@hwillson hwillson added the 🚧 in-triage Issue currently being triaged label Jul 9, 2019
@hwillson
Copy link
Member

hwillson commented Jul 9, 2019

This should no longer be an issue in apollo-client 2.6.8. Thanks!

@hwillson hwillson closed this as completed Jul 9, 2019
@blasterpistol
Copy link

@hwillson the latest version is 2.6.4 😄where I can find 2.6.8?

@hwillson
Copy link
Member

Oops - good catch @testarossaaaaa! When I typed that comment I meant 2.6.3, but this should still be fixed (🤞) in 2.6.4. Thanks!

@sebastienbarre
Copy link

Still an issue here for me with 3.2.4 when subscribing to watchQuery, my subscription's completeCallback is never called. Looking at the Observable Query code, I don't even see a call to complete()

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🚧 in-triage Issue currently being triaged
Projects
None yet
Development

No branches or pull requests

5 participants