apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.34k stars 2.66k forks source link

[Docs] refetchQueries can fetch non active queries #11894

Open maon-fp opened 3 months ago

maon-fp commented 3 months ago

Issue Description

In the docs it states:

You can only refetch active queries.

But one can pass an arbitrary query to the refetchQueries and apollo client will query and cache the results.

So either the docs are not correct or the client is "overfetching" not active queries.

Link to Reproduction

https://codesandbox.io/p/devbox/busy-maria-gjhpgz

Reproduction Steps

Go to the sandbox. Open the devtools. Add a person twice and you'll see the refetched animals in the console.

@apollo/client version

3.10.4

jerelmiller commented 3 months ago

Hey @maon-fp 👋

It looks like you're using the object-based syntax in refetchQueries, which does allow you to pass arbitrary queries to refetchQueries. This is currently undocumented and listed as "legacy" in our codebase though, so I believe this is a discouraged pattern (though to be honest, I'm not sure the historical reasons why).

https://github.com/apollographql/apollo-client/blob/a739dfd2847d002405bb6c8b637ead9e54d5d012/src/core/QueryManager.ts#L873-L875

The "active" queries refer to the DocumentNode and string-based inputs. I'll see if I can dig up any information on the historical reason why this is "legacy" since this change was made before my time on the team.

maon-fp commented 3 months ago

Thanks for your answer. I was just irritated by the docs not matching my observed behavior.

https://github.com/apollographql/apollo-client/blob/a739dfd2847d002405bb6c8b637ead9e54d5d012/src/core/QueryManager.ts#L873-L875

The "active" queries refer to the DocumentNode and string-based inputs. I'll see if I can dig up any information on the historical reason why this is "legacy" since this change was made before my time on the team.

As far as I understand the DocumentNode I'm using one in my example:

const ALL_ANIMALS = gql`
  query AllAnimals {
    animals {
      id
      name
    }
  }
`;
Cellule commented 3 months ago

I dug a bit into this and the problem is that the code here doesn't exclude "inactive" queries when trying to refetch queries by name https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/QueryManager.ts#L891-L896

It should likely instead be the following

        if (
          fetchPolicy === "standby" ||
-         (include === "active" && !oq.hasObservers())
+         (include !== "all" && !oq.hasObservers())
        ) {
          return;
        }

That way if the query is "inactive" (aka has no observers) then it would not be included in the refetch

However, if the query is not found it would warn later that it didn't it. Which is questionable https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/QueryManager.ts#L934-L945

In my case I often mean "refetch IF it active, otherwise, meh"

jerelmiller commented 3 months ago

@maon-fp apologies, coming back to this, I realize I misread your reproduction. You're right that the DocumentNode case should only work with active queries.

Good find on the code @Cellule. You're right, technically an inactive query won't be excluded if you're passing the query by name.

To be totally honest though, I'm not entirely sure the case where a query can become inactive and participate in the refetch, mostly because as soon as an ObservableQuery no longer has observers, we tear down the ObservableQuery

https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/ObservableQuery.ts#L152-L154

which calls stopQuery

https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/ObservableQuery.ts#L1057

which ultimately ends up removing that query from the list of queries (see line 1068):

https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/QueryManager.ts#L1049-L1070

This Map of queries is the same one that refetchQueries uses to compare against:

https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/core/QueryManager.ts#L879

So theoretically is should only be active queries it can use since those are the only ones that should be available in that Map of queries.

@maon-fp I tried your reproduction again and wasn't able to see the animals query fetched after the mutation. Each time I run the mutation, I'm seeing the warning about not being able to find a query that matches that document node. Is there something I'm missing here? I'd love to be wrong here and understand if there is a case where an inactive query can be refetched so we can update the docs accordingly. Let me know what I can do to see that fetch happen!

Cellule commented 3 months ago

I can guarantee there are inactive queries left in that list. See #11914 for a related issue with a repro