Apollo-client: Reactive variable value changes do not automatically update a dependent field.

[pranav-xx]

I am using ApolloClient 3.0.0-rc.12 for the first time and trying to learn reactive variables as described in documentation

Trying to achieve bookmarking blogposts using reactive variables (local state) in a blogging sample app.
Pardon me if this issue is not a valid issue and a bad usage instead.

Definition of reactive variable:

• BookmarkedPosts
• Underlying Generic class
• Field Definition
• Reactive variable value is getting changed here

Intended outcome:
Expecting the query using this field to automatically update and hence isBookmarked field should get updated

Actual outcome:
After clicking on bookmark icon, the value is correctly set in Reactive variable "bookmarkedPosts" but the query field dependent on this is not refreshing.

How to reproduce the issue:
clone repo
npm install
npm run build
npm run start

This should load homepage displaying bunch of posts cards. Each post has a small bookmark pin on top right corner.
Clicking this pin should toggle from light grey to dark grey

Versions

System:
OS: macOS 10.15.5
Binaries:
Node: 12.4.0 - ~/.asdf/installs/nodejs/12.4.0/bin/node
npm: 6.9.0 - ~/.asdf/installs/nodejs/12.4.0/bin/npm
Browsers:
Chrome: 83.0.4103.116

[elcssmouhsine]

same issue happend for me, queries do not rerun when i change reactive-variable from a react hook

[benjamn]

First of all, I love what you're doing with the LocalEntity abstraction! Reactive variables are definitely intended to be wrapped and augmented in this way.

As a rule of thumb, it doesn't matter where you change the reactive variable, but it does matter where you read the variable, and whether it's actually getting read in the places you intended.

In @pranav-xx's app/reproduction (thanks for sharing that!), the problem seems to be that getFieldReadDefinition returns an object with field names (like bookmarkedPosts) as keys, and that object gets ...spread into the Query type policy, here:

const cache = new InMemoryCache({
    typePolicies: {
        Post: {
            fields: {
                isBookmarked: (_, { readField }) => {
                    const id = readField<string>('id');
                    const bokmarks = BookmarkedPosts.get();
                    return bokmarks.has(id || '');
                }
            }
        },
        // Note added by @benjamn: I believe another (unrelated) problem with the reproduction
        // is that this field policy is named "Queries", when the default `__typename` for the root
        // query object is almost always "Query", so I've taken the liberty of using "Query" in these
        // examples, to avoid any confusion.
        Query: {
            ...BookmarkedPosts.getFieldReadDefinition()
        }
      }
});

If you compare this with the Post.isBookmarked field policy (which looks good to me), you'll notice there's an object called fields that comes between the Post type and its isBookmarked field (and any other fields). This is to be expected since the type definition for TypePolicy looks like this:

export type TypePolicy = {
  keyFields?: KeySpecifier | KeyFieldsFunction | false;
  fields?: {
    [fieldName: string]:
      | FieldPolicy<any>
      | FieldReadFunction<any>;
  }
};

However, in the Query type policy, the fields object has been lost, so the structure ends up looking like

        Query: {
            bookmarkedPosts() {...}
        }

when it should look like this:

        Query: {
            fields: {
                bookmarkedPosts() {...}
            }
        }

Although you could fix this in a couple of different ways, I think getFieldReadDefinition should stay the same, and you should just update the Query policy to have the fields object (rather than, say, making getFieldReadDefinition return { fields: { ... }}). Also be sure to rename it to Query instead of Queries, as explained in the comment above.

I'm not entirely sure why TypeScript can't provide more help in preventing this kind of mistake, but the result is that there is effectively no read function for the Query.bookmarkedPosts field, so the reactive variable never gets read by queries like query { bookmarkedPosts @client }, and thus those queries are not invalidated when the reactive variable is modified.

[benjamn]

Just in case my explanation above was confusing, here's a quick PR with the changes I'm recommending: https://github.com/pranav-xx/apollo-type-graphql-blogger/pull/1

[pranav-xx]

Thanks a ton @benjamn for explaining in detail and raising a PR for the same. I definitely missed to verify that

1. "Queries" had to be "Query"
2. and the read function was not getting wrapped under "fields".

Thanks for pointing that out.

I also found one more issue with the way I was setting value. I was mutating the value of reactive variable. Instead I should keep it immutable for it to know there was a change. Changes here