apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

[Docs] refetchQueries can fetch non active queries

Open maon-fp opened this issue 1 year ago • 5 comments

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

maon-fp avatar Jun 13 '24 08:06 maon-fp

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.

jerelmiller avatar Jun 13 '24 17:06 jerelmiller

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
    }
  }
`;

maon-fp avatar Jun 14 '24 06:06 maon-fp

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"

Cellule avatar Jun 28 '24 20:06 Cellule

@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!

jerelmiller avatar Jun 28 '24 21:06 jerelmiller

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

Cellule avatar Jun 28 '24 21:06 Cellule

I'm also seeing the same issue that inactive queries are getting refetched when just passing a DocumentNode to refetchQueries. Calling client.getObservableQueries() doesn't include these queries that still get refetched. The queries are still present in client.queryManager.queries though.

levrik avatar Dec 16 '24 08:12 levrik

After digging a bit deeper I realized that my queries are duplicate in queryManager.queries for some reason. Queries get correctly removed by queryManager.stopQuery() but these duplicates aren't.

levrik avatar Dec 16 '24 09:12 levrik

I think what I'm seeing is https://github.com/apollographql/apollo-client/issues/9903

levrik avatar Dec 16 '24 09:12 levrik