When variables change, data is the previous result in Query Component

[asafigan]

I am using the "cache-and-network" policy.

Intended outcome:
Rerender Query with different variables. Data is {} until new request is resolved from cache or network.

Actual outcome:
Rerender Query with different variables. Returns previous data until next query is resolved without error.

How to reproduce the issue:
https://stackblitz.com/edit/react-snlwzf
change the input and hit submit.

Version

• apollo-client@
• react-apollo@

• has-reproduction
• feature
• docs
• blocking
• good first issue

Backgroud
I only want to display a loading state if I don't have a response from the cache or network, but I do want to show loading state when variables change and I don't have a response from cache. Right now it is impossible to tell if data is the response from cache or if it is the previous result. Loading property is no help because it is allows true until the network request is resolved.

[jcready]

Yeah, you basically have to use something like this to prevent rendering data from a query with different variables:

if (data.reddit && data.reddit.subreddit.name !== this.state.search) {
  data = {};
}

And that only works if the data being returned also includes the variable you passed in. Definitely seems like a bug.

[sbrichardson]

This is a pretty common issue. You need to do a check for equality of a name/id and show a loading component or pass an empty/default obj if you don’t want to see stale data. I would advise modifying the data variable, if that’s the same one from the query result obj. Just use a ternary or if/else. You may need to abstract it to an external component to ensure you get rerenders reliably.

…

On Wed, Jul 25, 2018 at 10:45 AM James Wyatt Cready-Pyle < ***@***.***> wrote: Yeah, you basically have to use something like this to prevent rendering data from a query with different variables: if (data.reddit && data.reddit.subreddit.name !== this.state.search) { data = {}; } And that only works if the data being returned also includes the variable you passed in. Definitely seems like a bug. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2202 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIM6lxQm220hAw84gsU2XQj043KYvC17ks5uKKCzgaJpZM4VZucg> .

[jcready]

@sbrichardson the problem is that OP wants to see cached data for query when the variable is the same, but the problem is that Apollo is displaying the cached results for wrong query if the cached results for the current query do not already exist in the cache.

The expected behavior is for Apollo to provide an empty data object when the results for the current query do not exist in the cache.

The actual behavior is that Apollo is providing the previous data object when the results for the current query do not exist in the cache.

Please see a more detailed example here.

[asafigan]

I do think that it can be valuable to show the previous result while the new one is loading. I have a search auto complete feature that does this. But I also need this case. I can't check for equality of a name or id because the query is a search query. The variables that are passed into the function child are the same as the current input so I can't check equality using that.

[mike-marcacci]

@jcready - thanks for the clear description. I suppose that both behaviors could be desirable in different situations, and it’s a matter of which is default.

With the suggested strategy it would be pretty pretty trivial to restore the current behavior by creating a component stores the Query’s variables/results in state and only updates them once the variables have changed and data is present.

Changing the existing behavior in userland is a bit trickier, given that we don’t know which variables the current data is from.

I agree that this is a bug.

[jcready]

This issue also looks to be duplicated in #1342 and #1963

I believe the source of this issue is around this bit of logic in the <Query/> component:

react-apollo/src/Query.tsx

Lines 391 to 400 in 2788981

	if (loading) {
	Object.assign(data.data, this.previousData, currentResult.data);
	} else if (error) {
	Object.assign(data, {
	data: (this.queryObservable!.getLastResult() || {}).data,
	});
	} else {
	Object.assign(data.data, currentResult.data);
	this.previousData = currentResult.data;
	}

Specifically line 392. Since a query with different variables will not have cached data, but will have previous data from the previous query with different variables, the data will end up being just the previous data. I would argue that solution is to reset this.previousData in the componentWillReceiveProps() method when shallowEqual(nextProps.variables, this.props.variables) is not true.

To support pagination perhaps it is necessary to have the component accept some new prop which would either accept an array of variable keys which would invalidate the previousData if changed or perhaps just accept a function which would act similar to a react component's shouldComponentUpdate(). I imagine something like:

<Query
  query={query}
  variables={variables}
  fetchPolicy="cache-and-network"
  shouldInvalidatePreviousData={(nextVariables, previousVariables) =>
    nextVariables.subreddit !== previousVariables.subreddit
  }/>

[asafigan]

I agree that shouldInvalidatePreviousData should be a function that defaults to shallow equal. I implemented a simple replacement of the Query component that used that strategy and it worked well.

[marano]

I've just stumbled across this bug. No news if this is going to be fixed?

[brunoreis]

This is very weird. I'm almost sure I've seen this working well before in another app, but I'm facing this now too. It is most likely a matter of how the query is being cached. The cache's PK needs to be the query plus variables.

[brunoreis]

Looking at the cache I do see different entries for different variable sets:

[brunoreis]

first discovery here: if you use refetch instead of changing the props from the parent, the networkStatus (https://www.apollographql.com/docs/react/api/react-apollo.html#graphql-query-data-networkStatus) will be 2 that's more appropriate than 1. If you pass from the parent it will be one.

[brunoreis]

I agree with @jcready. That's probably the error due to the behavior I've observed here.

I spent a lot of hours yesterday trying to find a workaround and this is the best I could come up with. I prefer using the graphql hocs approach together with recompose functions. Please adjust the solution to your scenario.

The best way I could find to show correct data was to check if the query really has a cache and then to infer that the data is from the cache instead of from the previous variable set.

I created this method to check for the cache:

const queryCacheExists = (client, query, variables) => {
  try {
    client.readQuery({
      query,
      variables,
    });
    return true;
  } catch (e) {
    return false;
  }
};

And then this logic:

const hasItems = props.queryData.queryField ? props.queryData.queryField.edges.length > 0 : false;
      const queryLoading = props.queryData.loading;
      const showingCache = (
        hasItems
        && queryLoading
        && queryCacheExists(props.client, MY_QUERY, props.queryData.variables)
      );

So, after that, showingCache will indicate if the query is showing cached items. And then I can use a custom loading logic like:

loading: queryLoading && !showingCache,

If someone has any simpler solution, please let us know here on this issue.

[enriquemorenotent]

I suspect I have the same problem, but with Mutation. I posted it here.

[maximeg]

Encountered the same issue.

Solution:

• the proposed shouldInvalidatePreviousData can be a generic solution
• we could also simply not use previousData in the loading phase when fetchPolicy contains use of cache, because that's a double useless caching layer, with side effect as we can see here.

Relevent code:

react-apollo/src/Query.tsx

Lines 396 to 402 in 386b6d7

	if (loading) {
	Object.assign(data.data, this.previousData, currentResult.data);
	} else if (error) {
	Object.assign(data, {
	data: (this.queryObservable!.getLastResult() || {}).data,
	});
	} else {

[oeed]

Is there any work around for this or plans to fix it?

[fdhani]

any update on this issue, this is a pretty big problem imo

[adrianos10]

I also encountered this problem using HOCs. It only starts to work properly when fetchPolicy is set to 'no-cache'. Any update on this?

[UwUnyaa]

Just bumped into this problem. It seems as if Apollo looks for a similar object in the cache and uses that without checking any variables. When cache persistence is enabled, even refreshing doesn't affect it, and the only way around it is to disable cache for the query, which is unacceptable when offline functionality is desired.

I'd assume the client making a new query when variables changed and data for them isn't in the cache is a sane default.

[mssngr]

Encountering this, myself. I'll just have to turn off the cache for now, which is a bummer of a workaround.

[adamjv90]

Also encountering this issue.

[dunika]

I appreciate all the work done by the Apollo team but for me this is a very significant issue.

[TuHuynhVan]

I faced same issue and I put fetchPolicy: 'no-cache', that saved me after hours of debugging my React App.

        // Fetching new data for the PO Entity
        let pageObjectQueryResult = await client.query({
            query: getPageObjectEntityQuery,
            variables: {
                name: event.value
            },
            fetchPolicy: 'no-cache'
        });

[UwUnyaa]

@TuHuynhVan that's a workaround which won't work for everyone. In my case, being able to cache queries is critical as I'm working on a PWA with offline functionality.

[hwillson]

react-apollo 2.5.2 has a bug in it that prevents certain Query component updates from happening. This issue has been fixed in #2840, and will be published soon (hopefully tonight / tomorrow). I have a feeling this will help fix some of the issues people have been encountering here.

[jcready]

@hwillson #2840 will not solve the problem.

[hwillson]

@jcready It won't solve this entire issue thread, I agree. But I have a feeling it will help address some of the comments posted here in the past 2 weeks.

[IgorCRD]

@hwillson,
Version 3.1.0 seems to still have the same behavior.

To me we already have three solid approaches on how to solve this problem:

1. Adding a clearPreviousData flag as a query argument;
2. Adding a isPreviousData flag on the results object;
3. Adding a shouldClearPreviousData function argument, as proposed by @jcready;

The first approach is my favorite because is more of a declarative/reacty way of doing things.

I guess the first two approaches would attend most of the use cases, but it does not invalidate the use cases solved by the third and I think after implementing one approach the other two would be trivial (guessing out of complete ignorance sorry if it is harder than it seems). The hooks version could have an array containing values that would cause the hook to clearPreviousData, it would work like the hooks version of the third approach.

Just brainstorming here to let you all know that this problem is still a thing and to get the discussion running again.

[markplindsay]

@hwillson, I am experiencing this problem when using useQuery in version 3.1.1 as a part of a Next.js app. I'm a new Apollo user and this is the first issue I've had with this excellent software.

1. Component renders, and useQuery fetches results using variables {"currency": "INR"}
2. Variables change to {"currency": "CAD"}
3. useQuery correctly fetches and returns results using new variables
4. Variables change back to {"currency": "INR"}
5. useQuery incorrectly returns results as if variables were still {"currency": "CAD"}

Setting fetchPolicy: 'no-cache' as a useQuery option fixes it. Let me know if I can provide any more info.

[markplindsay]

@hwillson, my problem was actually the result of me not understanding how InMemoryCache works. There is no bug.

The {"currency": "INR"} in my comment above was actually part of a larger structure that looks like this:

{
  "search": [
    {
      "id": 321,
      "mpn": "NE555",
      "median_price_1000": {
        "currency": "INR",
        "price": 0.123,
        "__type": "PricePoint"
      },
      "__type": "Part"
    },
    {
      "id": 654,
      "mpn": "NE555P",
      "median_price_1000": {
        "currency": "INR",
        "price": 0.456,
        "__type": "PricePoint"
      },
      "__type": "Part"
    },
    {
      "id": 987,
      "mpn": "NE555R",
      "median_price_1000": {
        "currency": "INR",
        "price": 0.789,
        "__type": "PricePoint"
      },
      "__type": "Part"
    }
  ]
}

This over-simplified example was hiding the real cause of the problem. PricePoint doesn't have an ID, and so InMemoryCache was storing only the most-previously-fetched version of median_price_1000 for each Part as a part of its normalization process. It was treating all of a Part's PricePoint as being the same.

For anyone else who may be experiencing similar issues, by passing a dataIdFromObject function to InMemoryCache when initializing ApolloClient, I was able to append median_price_1000.currency to each Part's key to differentiate Part in the cache:

import withApollo from 'next-with-apollo'
import ApolloClient, { InMemoryCache, defaultDataIdFromObject } from 'apollo-boost'

export default withApollo(
  ({ initialState }) =>
    new ApolloClient({
      cache: new InMemoryCache({
        dataIdFromObject: (o: any) => {
          const id = defaultDataIdFromObject(o)
          if (o.__typename !== 'Part' || id === null) {
            return id
          }
          if (o.median_price_1000 === null) {
            return id
          }
          return `${id}:${o.median_price_1000.currency}`
        }
      }).restore(initialState || {}),
      ...
    })
)

This solved the caching problem without using fetchPolicy: 'no-cache'. There is probably an even better way to do this, but this works and is much better than disabling the cache entirely.

[UwUnyaa]

@markplindsay The problem is that Apollo should warn you in development mode when your dataIdFromObject() returns wrong values that cause problems with serialization.

[macrozone]

@hwillson can confirm this happens with "@apollo/react-hooks@^3.1.3"

useQuery returns old data when variables change (with loading: true)

Edit: but maybe that is intentional and could make sense in some cases

[joshjg]

@hwillson
Currently useQuery returns variables alongside data - as a user I'd expect these variables to be those used to fetch the old data (while new data is loading). However, they seem to be the same variables as those in the options argument, which doesn't seem very useful.

[Daniel-Griffiths]

Same problem here, the data is always "one version" too old.

[psamim]

Same here.

[afreix]

Has anyone found a suitable workaround for this?

[declanelcocks]

@afreix the trick of adding a key to the component was one approach that worked for me. Put one of the variables as the key and it should trigger the querying component to re-render.

[sbcreates]

@declanelcocks could you elaborate?

I'm having the same issue as others. I have a context in my React app that, when it changes, the id variable I have in my Query is changed and a new query should be called followed by updating the data.

When my context changes to a new variable, a query if executed and new data is populated. If I change my context back to a previously fetched variable, the id variable in the query gets updated but the query is not executed.

If I change my fetchPolicy to no-cache the query works as expected and refetches data every time my context is updated.

[richardscarrott]

Yeah also experiencing this issue in 3.1.0 and 3.1.1.

[deresegetachew]

I am facing the same issue with useLazyQuery. My Scenario is I am using fetchMore to load more data and the fetchMoreResult , variables variables inside updateQuery of fetchMore are different from that off onCompleted.

The order of Execution is as follows:

FetchMore -> UpdateQuery({fetchMoreResult , variables}) -> then OnCompleted is Called.

variables and fetchMoreResult in UpdateQuery are up to date; but the data parameter in Completed is not updated.

FYI it doesnt matter if my fetch policy is fetchPolicy:'network-only',

[mhuconcern]

This still occurs on v2.6.8 for me which is a stable build. I think I have spent an honest 4 hours trying to fix this bug where

<Query 
variables={{this changes}}
fetchPolicy='network-only'/>
// Check network status, it's always 7
// Data is fulfilled twice, once with older param second with newer param, 
// causing intermittent ui flashes of data
</Query>

[trdaya]

This still occurs on v2.6.8 for me which is a stable build. I think I have spent an honest 4 hours trying to fix this bug where

<Query 
variables={{this changes}}
fetchPolicy='network-only'/>
// Check network status, it's always 7
// Data is fulfilled twice, once with older param second with newer param, 
// causing intermittent ui flashes of data
</Query>

@mhuconcern I am facing the exact same issue with the version you mentioned

[mhuconcern]

This still occurs on v2.6.8 for me which is a stable build. I think I have spent an honest 4 hours trying to fix this bug where

<Query 
variables={{this changes}}
fetchPolicy='network-only'/>
// Check network status, it's always 7
// Data is fulfilled twice, once with older param second with newer param, 
// causing intermittent ui flashes of data
</Query>

@mhuconcern I am facing the exact same issue with the version you mentioned

@trdaya #321 (comment) . If this helps you. I realize that this bug even though it exists on this build, it is exposed more if your graphql apis don't have a way to define if the returned object is unique to the request param

[trdaya]

This still occurs on v2.6.8 for me which is a stable build. I think I have spent an honest 4 hours trying to fix this bug where

<Query 
variables={{this changes}}
fetchPolicy='network-only'/>
// Check network status, it's always 7
// Data is fulfilled twice, once with older param second with newer param, 
// causing intermittent ui flashes of data
</Query>

@mhuconcern I am facing the exact same issue with the version you mentioned

@trdaya #321 (comment) . If this helps you. I realize that this bug even though it exists on this build, it is exposed more if your graphql apis don't have a way to define if the returned object is unique to the request param

@mhuconcern thanks 👍 . I tried this but unfortunately, it didn't work for me.
Some assumptions and findings:
Since the fetchPolicy is cache-and-network:

1. It returns the data (if any) from the cache. networkStatus is 7 if the data (stale data) is already present in the cache
2. It makes a network call in parallel to step 1, to fetch the latest data, to keep the cache up to date
3. When the network call completes and the cache is updated, new data is pushed to the client. networkStatus 7 (this time because data fetching from network is completed) with new data.

I tried throttling my network on chrome's dev-tools and found that this issue doesn't happen on slow networks. networkStatus 7 is received only once, with the new data.

[WillSquire]

Just incase anyone else is having issues working around this, comparing current data to previousResult in updateQuery works for me. E.g.:

return fetchMoreResult.length === 0 || data !== previousResult
          ? previousResult
          : // add new data

[sarojkumar007]

This is a pretty common issue. You need to do a check for equality of a name/id and show a loading component or pass an empty/default obj if you don’t want to see stale data. I would advise modifying the data variable, if that’s the same one from the query result obj. Just use a ternary or if/else. You may need to abstract it to an external component to ensure you get rerenders reliably.
…
On Wed, Jul 25, 2018 at 10:45 AM James Wyatt Cready-Pyle < @.***> wrote: Yeah, you basically have to use something like this to prevent rendering data from a query with different variables: if (data.reddit && data.reddit.subreddit.name !== this.state.search) { data = {}; } And that only works if the data being returned also includes the variable you passed in. Definitely seems like a bug. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2202 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AIM6lxQm220hAw84gsU2XQj043KYvC17ks5uKKCzgaJpZM4VZucg .

Worked for me. Thanks

[krvajal]

In my case I am doing the following

  let { data, loading, fetchMore, networkStatus } = useQuery<GetCompanyLeadsQuery, GetCompanyLeadsQueryVariables>(GET_COMPANY_LEADS_QUERY, {
        variables: {
            type: ContactType.Vendor,
            query: queryField.value,
            page: 1,
        },
        notifyOnNetworkStatusChange: true,
    });

    if(networkStatus === NetworkStatus.setVariables) {
        data = undefined;
    }

So when the query variable changes, the network status is set to setVariables and I set data to undefined to avoid rendering stale result for another query.

[seeruk]

@krvajal As far as I can see, the problem there is that if you do have the data in the cache you end up wiping it out then with that approach.

The only approach I've currently seen working is somehow echoing some of the data included in your query variables in the data so you can see if the data is new or not.

This is a huge issue for applications that want to show up-to-date data, but still respond extremely quickly and opportunistically, and I've not seen a good solution that doesn't rely on using the response data (which isn't always possible).

[lloydaf]

This is proving to be such a big pain. Trying to fit in rxjs into the mix is fun(not). After spending a lot of time working with Angular, it's amusing to me that useLazyQuery doesn't even return a promise at this point. Maybe it's just me.

[Elijen]

I am considering leaving development job and starting to work as a bus driver because of this bug.

[codefreak13]

I was able to solve this by using a combination of client.resetStore() on my logout function and fetchPolicy='network-only' on my Query.

[Javey]

I use @client directive to set a local value, when I have mutated it, Apollo will update it in next Microtask, so I read the previous result.

new QueryData({
    options: updatedOptions,
    context: context,
    onNewData: function () {
        if (!queryData.ssrInitiated()) {
            // update next tick
            Promise.resolve().then(forceUpdate);
        }
        else {
            forceUpdate();
        }
    },
});``