urql-graphql / urql

The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
https://urql.dev/goto/docs
MIT License
8.54k stars 444 forks source link

Cache update and invalidation in same transaction voids the update #3618

Open BickelLukas opened 2 weeks ago

BickelLukas commented 2 weeks ago

Describe the bug

I am executing a query that updates creates a calendar event. As a result I am getting back that event instance and adding it to the list via an updater. This far everything works great. However, creating a event may also add other events in the background that are not immediately returned (think recurring events). So what I want to do is:

Now I have the following updater:

createCalendarEvent(result, args, cache) {
  if (result.createCalendarEvent.calendarEventInstance) {
    cache
      .inspectFields("Query")
      .filter((field) => field.fieldName === "calendarEvents")
      .forEach((field) => {
        const items = cache.resolve("Query", field.fieldName, field.arguments);
        if (Array.isArray(items)) {
          cache.link("Query", field.fieldName, field.arguments, [...items, result.createCalendarEvent.calendarEventInstance]);
        }

        // cache.invalidate("Query", field.fieldName, field.arguments); <-- Adding this breaks the update
      });
  }
},

As soon as I add the invalidate in the updater, the manual updates are no longer visible and the event does not get added until the refetch completes resulting in flashing of the UI.

In my opinion this should work but if that behaviour is on purpose please advice on how else this should be solved.

(I wanted to create a repro but the Postgres db for https://trygql.formidable.dev/ is down. If needed I can provide one later when its up again)

Reproduction

See description

Urql version

"urql": "^4.0.7" "@urql/exchange-graphcache": "^7.0.2",

Validations

JoviDeCroock commented 2 weeks ago

This is indeed intended behavior, basically what you're doing is not marking it for a "refetch" but removing the whole connection. Marking it for a refetch would be that you invalidate a nullable part of the operation and that triggers a refetch while displaying stale data.

What you are doing here is that you add an entity to your Query.fieldName(...arguments) and then you immediately set that Query.fieldName(...arguments) to undefined (as that is what invalidate essentially does). This means that when we re-query the data rather than finding the added item we'll find undefined which means we have to initiate a refetch rather than give you the updated data. We won't even be able to discover the item added there because the Query.fieldName(...arguments) does not point at anything.

BickelLukas commented 2 weeks ago

Thanks for the quick answer. How come when I invalidate data, it is still displayed in the UI until the refetch returns a new result?

Is there any way to achieve the desired result (mark for refetch) with graphcache?

JoviDeCroock commented 2 weeks ago

A "hack" for this would be to invalidate a property that is nullable on an entity contained within the lists you want invalidated. This will allow graphcache to query all the properties, apart from the ones marked as nullable and refetch

Scrap that, even that won't work 😅 I don't think there's currently a way to observe that, all though in theory subscriptions were created for things like this... Or you can try polling...

BickelLukas commented 2 weeks ago

We were able to fix our problem by manually refetching the query in the component when an event gets created. However I do still think that "adding and marking for refetch" is a valid use case for graphcache. Do you plan on supporting something like that in the future?

kitten commented 1 week ago

You might be better off splitting this result up or modelling your mutation differently.

Basically, updating is an all or nothing approach unless you trigger query updates in another way. That means that updates are intended to fully reflect either invalidations or updates. In other words, because a query can either be a cache miss, cache hit, or a partial hit, you cannot invalidate data for it and still receive it per se.

You basically need a signal at the query end.

There's several ways though of achieving what you're looking for:

Basically, the reason why you're running into trouble is because this case is fighting the cache in a way. You have a mutation that updates, but the update is redundant because you're trying to also invalidate. In a way, you already are trying to force two requests to be happening when you invalidate, so this is unsupported because in cases like these there's no point in preventing this from happening for the cache.

Personally, I'd try to model this as an optimistic and non-optimistic update, since this gives the user immediate feedback.

Re. the flashing of UI, this really depends on how you treat refetches. This basically doesn't have much to do with the updated but with how you treat these refetches. If you hide the UI while a refetches occurs with result.stale, you'll always get the flash, since refetches like these are already "backgrounded", i.e. your useQuery bindings won't lose their state during these background fetches