Urql: Using cache.updateQuery() data is null

[frederikhors]

urql version & exchanges:

Steps to reproduce

1. go here: https://codesandbox.io/s/urql-svelte-crud-with-indexeddb-cqg5i?file=/urql.js:1152-1179
2. click on "Todos list"
3. click on a "Complete" button, the one you want
4. it gives an error: Cannot set property 'text' of null

Why this error?

Why is data null?

I'm using this code:

import { initClient, dedupExchange, fetchExchange } from "@urql/svelte";
import { offlineExchange } from "@urql/exchange-graphcache";
import { makeDefaultStorage } from "@urql/exchange-graphcache/default-storage";
import gql from "graphql-tag";
import { TODOS_QUERY, TODO_QUERY } from "./gql";

const updates = {
  Mutation: {
    updateTodo: (result, args, cache, info) => {
      if (info.optimistic) {
        cache.updateQuery(
          { query: TODO_QUERY, variables: { id: info.variables.id } },
          data => {
            console.log("data:" + data);
            data.text = "UPDATING";
            return data;
          }
        );
      }
    }
  }
};

const optimistic = {
  updateTodo: () => {}
};

const storage = makeDefaultStorage({
  idbName: "graphcache-v3",
  maxAge: 7
});

const exchanges = [
  dedupExchange,
  offlineExchange({ storage, updates, optimistic }),
  fetchExchange
];

function OnInit() {
  initClient({ url: "https://h1pcl.sse.codesandbox.io", exchanges });
}

export default {
  OnInit
};

[JoviDeCroock]

Because you have never queried a TODO_QUERY yet at that point so for all the cache knows a TODO_QUERY might be returning something with __typename: Author it can't assume anything about your data since that would lead to errors.

Your safest bet in that scenario is to utilize writeFragment which can safely assume this.

        cache.writeFragment(
          gql`
            fragment _ on Todo {
              id
              text
            }
          `,
          {
            id: args.id,
            text: "UPDATING"
          }
        );

I do wonder why you are doing an optimistic update there, you could safely do

optimistic: {
  updateTodo: (args) => ({ id: args.id, __typename: 'Todo', text: 'UPDATING' })
}

[frederikhors]

You're right.

But you know what? Before offlineExchange (with cacheExchange only) I never had problems!

Can I ask you why? Same code.

[JoviDeCroock]

Because you likely were using a resolver pattern, but we opted to not follow resolvers in write scenario's to increase data consistency

First change here: https://github.com/FormidableLabs/urql/blob/master/exchanges/graphcache/CHANGELOG.md#patch-changes

[kitten]

Yep basically the thing we’re encouraging is atomic updates as often as possible, which increases consistency and reduces the amount of code that needs to be written or shared with the cache.

We’re also looking into a more lowlevel write primitive that is similar to cache.resolve but for writing rather than reading data fields.

[frederikhors]

So what do you suggest for now?

I keep using cache.writeFragment?

[frederikhors]

@JoviDeCroock I remembered why before offlineExchange I was not using optimistic that way.

The problem is this:

Can I somehow remedy those warnings?

In this example online there are few, in my real case they are many more!

[JoviDeCroock]

That means your mutation is expecting that value but you are not giving it which seems accurate given your example

[frederikhors]

This is a good example for discussing it.

Sometimes I don't need to pass all the fields to the server, because maybe I'm just changing one, as in this case (complete).

Among other things, field like createdAt doesn't even need to be sent to the server because I can't updtae them on the client.

How you do it?

[JoviDeCroock]

It relates to the data you ask back from the server

updateTodo {
  createdAt
  completed
}

Means you want these as optimistic values as well, we can't decide for you what should and should not be optimistic. It has nothing to do with the arguments pass to the server, it's the arguments you ask from the server

[frederikhors]

I was not clear.

I meant to say that I use the same fragment at different areas in the application.

Sometimes I input all fields (even createdAt) at other times not.

And in optimistic I use this code:

updateTodo: args => ({ ...args.input, text: "UPDATING", id: args.id, __typename: "Todo" }),

Sometimes args.input does contain all fields, sometimes not.

This is my problem.

Do we necessarily need those warnings?

Is there any problem using the app if there are undefined fields?

Is there a better way to get around this?

[kitten]

Also, to make this more complete, if your UI handles optional fields properly, it’s always possible to set these fields to null if they are indeed optional.

But the requirements of the optimistic mutation data will always have to match your selection set because that’s what will be written optimistically and emulate the server result.

Lastly, if those fields are undefined or missing, they’ll be skipped. So even seeing these warnings isn’t going to prevent you from defining the optimistic data. It’ll just likely be a mistake, since the query data won’t update fully. That’s why we warn about it. We could I suppose silence it if the field is explicitly set to undefined though

[JoviDeCroock]

There are no problems using the app, these are needed since they help a lot of people avoid pitfalls related to optimistic updates.

You can use the second argument to optimistic which is the cache to look for the missing fields

[frederikhors]

You can use the second argument to optimistic which is the cache to look for the missing fields

Is there an example?

[JoviDeCroock]

It's the same as you are doing in updates.Mutation, cache.readFragment with the TodoFragment, id and you should have that data

[frederikhors]

Okay, I ended up using this form:

const optimistic = {
  updateTodo: (args, cache) => {
    const actualTodo = cache.readFragment(
      gql`
        ${TODO_FRAG}
      `,
      { id: args.id }
    );
    return { ...actualTodo, ...args, text: "UPDATING" };
  },
}

However, I still don't know why, but I don't like it very much.

I'd like to avoid calling a readFragment every time (using gql) just because I want my UI to highlight an ongoing action.

It seems to me a "waste" of resources (considering that I have many optimistic around the app).

The warning is saying to me:

• "Look, this optimistic instead of responding as the client wishes is not sending all the data, are you sure it's correct?"

and I could reply:

• "Of course I'm sure, I just need to highlight that I'm saving, deleting or adding a todo; wait a moment and you will see that the server answers you correctly, it doesn't make sense to call thereadFragment complete with Graphql AST query filled in just because you want to be perfect, don't you think?".

Or am I wrong to use the optimistic and maybe there is another way to only highlight UI actions, what do you think?

[JoviDeCroock]

Optimistic updates are meant to reflect an early answer from the server, using to indicate something is updating isn't really its use case. This could be done by means of result.fetching instead.

The thing is, optimistic is used to ensure we can actually be offline, when a user is offline and performs a mutation just highlighting that certain thing is updating isn't of much use to our user, if it's unsafe to simulate an early response we should doubt that this entity can be mutated in an offline-state.

[frederikhors]

Thank you both.