apollo-feature-requests icon indicating copy to clipboard operation
apollo-feature-requests copied to clipboard

setVariables doesn't notify subscribers when cache hit

Open PowerKiKi opened this issue 7 years ago • 4 comments

Migrated from: apollographql/apollo-client#2712

Quoting myself:

About the potential behavior changes... in what kind of use-cases would it be preferable to watch the store, instead of watching the result for a public API ? From the consumer point of view it seems that the difference between store and result is very thin if not non-existent.

If there aren't any strong use-cases for a public API to watch the store, then I'd say the behavior should change to always watch the results instead. Making the store even more transparent for the consumer for most common use-cases.

This discussion should be about whether watchQuery should watch the store (as of now) or the result, and also whether it should notify subscribers with obsolete and incorrect results when notifying of loading status.

It seems @hwillson was open to changing this behavior in next major release.

PowerKiKi avatar Jul 28 '18 01:07 PowerKiKi

Here's my partial workaround for the problem. Unfortunately, I'm using fetch() because watch().valueChanges doesn't behave well when combined with pipe(startsWith()). So, this doesn't observe Store changes. It only updates when a variable is changed.

import { Query, Apollo } from 'apollo-angular';
import { switchMap, startWith, map } from 'rxjs/operators';
import { Observable } from 'rxjs';
import { ApolloQueryResult, NetworkStatus } from 'apollo-client';
import { Injectable } from '@angular/core';
import { QueryOptions } from 'apollo-angular/types'; // This is not the same type exported by 'apollo-client'!

export interface GqlQueryOptions extends QueryOptions {
  useStaleDataWhileLoading?: boolean;
}

@Injectable()
export abstract class GqlQueryBase<TQuery, TVariables> extends Query<TQuery, TVariables> {
  constructor(apollo: Apollo) {
    super(apollo);
  }

  private _priorData: TQuery;
  watchVariables(variables$: Observable<TVariables>, options?: GqlQueryOptions): Observable<ApolloQueryResult<TQuery>> {
    const usePrior = options && options.useStaleDataWhileLoading;

    return variables$.pipe(switchMap(variables => {
      return this.fetch(variables, options).pipe(
        map(res => {
          if (usePrior) {
            this._priorData = res.data;
          }
          return res;
        }),
        startWith({
          loading: true,
          data: usePrior ? this._priorData : null,
          networkState: NetworkStatus.loading,
          stale: usePrior,
        }),
      );
    }));
  }
}

chilltemp avatar Aug 28 '18 15:08 chilltemp

Trailing from: https://github.com/apollographql/apollo-client/issues/2712

I think we can do subscribe like this: this.rows.observer.setVariables(variables, true)

bhimeshchauhan avatar Oct 30 '18 20:10 bhimeshchauhan

No you shouldn't. You should never use setVariables() at all and instead use refetch() as is now documented: https://www.apollographql.com/docs/react/api/apollo-client.html#ObservableQuery.refetch

PowerKiKi avatar Oct 31 '18 01:10 PowerKiKi

I don't see the reasoning behind WHY we should use refetch over setVariables... because of the above issue?

It says:

Update the variables of this observable query, and fetch the new results. This method should be preferred over setVariables in most use cases.

What are MOST cases and which ones should we not use setVariables?

Not sure if it's an issue in apollo-angular but neither methods seem to work properly at all for me.

In this specific case I can't use either:

  • refetch only extends the current variables and doesn't replace, so if you go from: { enabled: true, features: ['LIVE'] } to { enabled: true } then it won't remove the features property because it's only extending and not replacing. I can't send features: [] because that has a different meaning in the API than not providing it at all
  • setVariables doesn't notify subscribers so I don't get the previous set of results when toggling the feature filter ON/OFF

Edit: it seems my solution was: { enabled: true, features: undefined } to force it to unset it

intellix avatar May 25 '20 17:05 intellix