apollographql / apollo-link-state

✨ Manage your application's state with Apollo!
MIT License
1.4k stars 100 forks source link

Retrieving remote data nulls out client side state #262

Open city41 opened 6 years ago

city41 commented 6 years ago

I am finding my local client data getting nulled out after making a query that involves a round trip to the server. But if the query is completely pulled from the cache, then the client side value does not get nulled.

This is a bit tough to explain, but I created a tiny app that repro's the issue: https://github.com/city41/apolloissue

It will stand up a tiny Express server and start a React app that has three buttons:

image

If you click the first button, get data with 'hello', it will run this Apollo query:

    query myQuery($bar: String!) {
        foo(bar: $bar) {
            id
            bar
        }
        clientValue @client {
            baz
        }
    }

Since the clientValue baz has not yet been set, you get this result and it's expected:

{
  "foo": {
    "id": "hello",
    "bar": "hello",
    "__typename": "Foo"
  },
  "clientValue": null
}

Then click set client value button, and the client value gets set, and we now see this result, also expected:

{
  "foo": {
    "id": "hello",
    "bar": "hello",
    "__typename": "Foo"
  },
  "clientValue": {
    "baz": "client value set",
    "__typename": "ClientValue"
  }
}

Now, click the third button get data with 'hiya', this will make another trip to the server using the same query as before, but with a different variable. This time this result comes back and it is not expected:

{
  "foo": {
    "id": "hiya",
    "bar": "hiya",
    "__typename": "Foo"
  },
  "clientValue": null
}

You can see clientValue got nulled out.

But now that both server side queries are in the cache, if you set the client value again, then from here on out the clientValue never gets nulled out.

Am I doing something wrong?

Apologies for a repro involving a server, but I could not repro this issue using only a client side app.

diegomura commented 6 years ago

I'm having the same issue. @city41 could you find a fix?

city41 commented 6 years ago

I suspect if I moved the client field to its own query it would probably be a workaround, but I've not tried it yet.

So basically have

<Query query={clientQuery}>
    <Query query={serverQuery}>
        ...
fracmak commented 6 years ago

I'm finding this issue after upgrading from 0.4.0 to 0.4.1. The order of operations is as follows. 1: Load default into cache 2: cache.writeData({ field: 'newvalue' }) 3: component rerenders with field set 4: network request is triggered 5: after network request finishes, component rerenders with the default value of field

It appears that there's a disconnect between where the initial default value is put in the cache "ROOT_QUERY" vs where cache.writeData puts the data "ROOT_QUERY.field", leaving ROOT_QUERY alone, and then depending on how you read from the cache, you may get stale data.

Reverting to 0.4.0 fixes the problem for me.

My problem is definitely related to #202. Appears that not defining a Query resolver for each type in the state sometimes causes the default value to be returned.

city41 commented 6 years ago

@fracmak on #202 you mentioned "if you don't define a query resolver". I wonder if another workaround (or possibly even the correct approach) is to always define a query resolver for client state

fracmak commented 6 years ago

what's weird for me is that the behavior is intermittent. So my UI freaks out a lot, where things revert to default state, then flip back to the cache state, only to flip back to default state depending on whether the UI is updating with network requests or not, or combined with other requests.

I'm wondering if there's an alternate solution to the problem that #202 was trying to fix. Isn't the purpose of this library to populate the apollo cache with a default set of values and then just forward all requests to that? I'm not sure why we need to arbitrarily return the default value just because we didn't find a Query resolver.

mbrowne commented 6 years ago

@city41 I tried defining a query resolver for my client-side state, but that does not fix the issue. Even when client.readQuery() (which reads the cache directly) returns the correct value, a query that requests both local and remote fields can still cause the default value to be returned instead.

And BTW, setting defaults has the same effect as writing resolvers to return the default values.

kmrachko commented 5 years ago

This issue is a huge problem for our application as we develop large-scale SPA which involves a lot of combining remote & local data. I hope it will be fixed soon or at least there will be some best practices of dealing with this.

For now, I see 2 options how one could fix this issue:

  1. The most simple one already described - split local and remote data fetching into separate queries. Straightforward, but ugly as one of the main features of this package is to be able to combine these types of data.
  2. Second is much trickier but seems to be working: as library resets to defaults only when no resolvers for local fields are present, you can add some resolvers that will look into the cache and check if the local data is there. Then return it in case it exists, or return a default state if not. This logic can be generalized and put into some shared resolver, but it is rather a hack than a proper solution.

A very similar problem is also related to augmenting remote types with local fields. When you refetch remote entity, all its local fields are being resolved again, which leads to resetting them into the default state (if you use resolvers like () => defaultValue). This also drives me nuts, but I came up with a somewhat working solution for that:


export default typeName => (fieldName, initialResolver) => {
    const fragment = gql`
        fragment ${fieldName}Fragment on ${typeName} {
            ${fieldName} @client
        }
    `
    return (root, args, context) => {
        try {
            const { id } = root
            const { cache, getCacheKey } = context
            const cacheId = getCacheKey({ __typename: typeName, id })
            const data = cache.readFragment({ id: cacheId, fragment })

            if (data === undefined || data === null || Object.keys(data).length === 0) {
                return typeof initialResolver === 'function'
                    ? initialResolver(root, args, context)
                    : initialResolver
            }

            return data[fieldName]
        } catch (e) {
            return typeof initialResolver === 'function'
                ? initialResolver(root, args, context)
                : initialResolver
        }
    }
}

What it does is simply checking if the data for this field is already present in the cache and if it is, return it. Otherwise - return some default state (can be provided via a function or direct value). That way we get resolver creator through currying for some specific type, and then we can create resolvers for local fields of that remote type (Bookmark in the example below):

const bookmarkResolverCreator = createResolver('Bookmark')
const resolvers = {
    Bookmark: {
            selected: bookmarkResolverCreator('selected', false),
            timeframe: bookmarkResolverCreator('timeframe', remoteData => calculateDerivedData(remoteData))
    }
}

Nothing fancy, but covers the needs of our project for now.

Still, this is a hack and I strongly recommend authors and contributors to pay attention to these issues.

mbrowne commented 5 years ago

Note: apollo-link-state will be a built-in part of Apollo 3 (Apollo has publicly announced this but I don't have the link handy at the moment). So I think all the attention and effort is going into that rather than this repo.

smolinari commented 5 years ago

link-state is in 2.5.0 @mbrowne.

Scott