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

Waiter not registered for some async calls #192

Open
viniciussbs opened this issue Dec 21, 2018 · 16 comments
Open

Waiter not registered for some async calls #192

viniciussbs opened this issue Dec 21, 2018 · 16 comments

Comments

@viniciussbs
Copy link
Contributor

When we face this issue:

There are some scenarios when settled does not wait for some async calls:

  • when there is a watched query and we refetch the query;
  • when there is a watched query and we fetch more data;
  • when we send a mutation with the option refetchQueries;

We just increment the ongoing counter for the first fetch. When we call refetch, fetchMore or set refetchQueries when sending a mutation, we do not register those ongoing requests.

How this is solved in my app

Query Manager

import QueryManager from 'ember-apollo-client/apollo/query-manager';

export default QueryManager.extend({
  refetch(opts, observable) {
    return this.apollo.refetch(opts, observable);
  },

  fetchMore(opts, observable) {
    return this.apollo.fetchMore(opts, observable);
  }
});

Service

import ApolloService from 'ember-apollo-client/services/apollo';
import QueryManager from 'my-app/apollo/query-manager';

export default ApolloService.extend({
  createQueryManager() {
    return QueryManager.create({ apollo: this });
  },

  refetch(opts, observable) {
    return this._waitFor(observable.refetch(opts));
  },

  fetchMore(opts, observable) {
    return this._waitFor(observable.fetchMore(opts));
  },

  mutate(opts, resultKey) {
    if (Ember.testing) {
      opts.awaitRefetchQueries = true;
    }

    return this._super(opts, resultKey);
  }
});

Examples of how we use those functions:

  • fetchMore for pagination
  • refetch for searches
  • mutation with refetchQueries for deletes

Disclaimer

We are not mocking our API. Our API has a sandbox server for tests. Maybe mocks are obfuscating this issue.

@tchak
Copy link

tchak commented Jan 27, 2019

It looks like you passing a promise to _waitFor where it expects a resolver – a function with (resolve: Function, reject: Function) signature.

@tchak
Copy link

tchak commented Jan 27, 2019

I will provide some context. ember-fetch implements a hook in to ember test waiting system. ember-apollo-client uses ember-fetch and as such will wait in tests for the network but there might be more asynchronous stuff going on inside apollo layer and it is also possible to use apollo-client with a link that have nothing to do with ember-fetch. For example, using plain fetch or a completely different transport. That's why ember-apollo-client also implements its own hook into ember test waiting system. This is what you trying to use in your extension and it should work (if you passe in a resolver).

@viniciussbs
Copy link
Contributor Author

@tchak where did you saw the signature of _waitFor? The documentation in the code says that it expects a promise. Or am I look at the wrong place?

@tchak
Copy link

tchak commented Jan 29, 2019

Hmm, you right, sorry. I am not sure why your implementation is not working then.

@viniciussbs
Copy link
Contributor Author

@tchak the issue is that ember-apollo-client does not cover those scenarios. This implementation I've posted here actually works. It's here as a reference so we can discuss if this is something that should be handled by this addon or on every app. I'm waiting an input from @bgentry about it so I can prepare a PR.

Sorry if the description is not so clear.

@tchak
Copy link

tchak commented Jan 29, 2019

Ah, cool! Sorry, I was confused because of the cross post in the ember-fetch issues.

@viniciussbs
Copy link
Contributor Author

@bgentry Any input on this one?

@viniciussbs
Copy link
Contributor Author

@josemarluedke Are you the new maintainer? Do you need with this issue?

@josemarluedke
Copy link
Member

@viniciussbs Yes, I'm helping maintain this project. Are you still having this issue? If so, could you create a pull request with a failing test? Or create a reproduction repo.

I also was looking forward to migrating to ember-test-waiters (see #297). Depending on the issue here, that could help to fix this problem.

@viniciussbs
Copy link
Contributor Author

@josemarluedke I'm not facing this issue because I'm using the solution I've posted here. But, yes, the problem still exists.

This is not a bug, it's just a case of "not supported feature" of Apollo Client. Apollo Client give us refetch and fetchMore functions and the refetcheQueries option for mutate - all of them to be used with watched queries - but ember-apollo-client does not register test waiters for those cases.

I'll try to send you a pull request with the failing test or create a reproduction repo.

@josemarluedke
Copy link
Member

I'm trying to get a reproduction of this issue and unfortunately, I'm not able to do so.

I have created a test for the refetch case and it works fine in our test suite. Could it be due to Ember versions? What version are you using?

A side note, I actually removed the code for the test waiters and all our tests still passes. I will add some tests to specifically make sure we have coverage for this.

@viniciussbs
Copy link
Contributor Author

@josemarluedke We are using Ember 3.7. I don't think it's related to the version, though.

A side note, I actually removed the code for the test waiters and all our tests still passes. I will add some tests to specifically make sure we have coverage for this.

Maybe that's it. Those tests use a mocked API, I guess. Ours hit a "sandbox API" - for a lot of reasons, we decided not to mock our API. Maybe you don't have failures because the mocked API responds right away? 🤔

@xtagon
Copy link

xtagon commented Sep 27, 2019

For what it's worth, I was using a sandbox API as well when I also encountered this issue. That's probably it.

@viniciussbs
Copy link
Contributor Author

@xtagon Could you try the solution that I've posted? It seems to work here.

@xtagon
Copy link

xtagon commented Sep 28, 2019

@viniciussbs Your solution is working for me, thank you :)

@mydea
Copy link
Contributor

mydea commented Nov 12, 2020

For reference, you can now do this pretty easily yourself using @ember/test-waiters. Just wrap any promise in your application code like this:

import { waitForPromise } from '@ember/test-waiters';

export default MyComponent extends Component {
  fetchMore() {
    return waitForPromise(this.apollo.fetchMore(...))
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants