Apollo-client: readQuery after cache evict

[danReynolds]

Hello! I have a question about how eviction of normalized objects interacts with queries that already had references to those objects. For example, if I were to do the following:

  const employeesQuery = gql`
    query {
      employees {
        data {
          id
          employee_name
        }
      }
    }
  `;

  const employeesResponse = {
    employees: {
      __typename: "EmployeesResponse",
      data: [{ id: 1, employee_name: 'Test', __typename: 'Employee' }],
    },
  };

cache.writeQuery({
  query: employeesQuery,
  data: employeesResponse,
});

I could then access that data like this:

cache.readQuery({
  query: employeesQuery,
});

but if I then evict the entity that the query contains:

cache.evict('Employee:1')

then subsequent calls to readQuery for employeesQuery fail with error Dangling reference to missing Employee:1 object and the query seems to now just be completely inaccessible and there doesn't seem to be a way to clear out the bad ref that it holds so that readQuery will work again. Is there something I'm missing?

it makes sense that the query would now have missing data, but there seems to be no way to remove it from the query since first I'd need to call readQuery to get what's currently there and then writeQuery to remove the dangling object. If it was able to return partial data then it would work, but reads prohibit that here: https://github.com/apollographql/apollo-client/blob/a18ee7ec9378bcf71ac6d37f76fbb8695f7ee565/src/cache/inmemory/readFromStore.ts#L108

This forces users to have to refetch queries that have had entities evicted, which can introduce unnecessary network burden if instead the user knows they can remove the evicted entity from that query.

Let me know what I should do to enable accessing or updating those stale queries, thanks!

[hwillson]

@danReynolds can you confirm which @apollo/client beta version you're using?

[danReynolds]

@hwillson this is on 3.0.0-beta.49. I think the read not supporting partial data is intended behavior, I'm just not sure how we can access fields containing dangling refs then. It'd be nice if evict removed dangling refs, although that might hurt performance so the easiest thing to do I think would be to not return dangling refs in the query response but still allow the query to succeed either with an optional returnPatialData flag, which would be possible if readQueryFromStore made it an option or by default

[danReynolds]

@hwillson Here's a commit off latest master demonstrating the problem: https://github.com/danReynolds/apollo-client/commit/d2a09a76d4972d9a559cba29ba2efd572fca7d87, if you run it you'll see it errors out with the dangling reference problem. The goal is to be able to make the query valid again by removing the evicted reference.

[dodas]

What about removing entities from queries before evicting them?
I thought the correct way is to update the cache to remove the items I don't want and then evict them / or gc the whole cache.

[danReynolds]

What about removing entities from queries before evicting them?
I thought the correct way is to update the cache to remove the items I don't want and then evict them / or gc the whole cache.

ah because that's more manual effort than necessary, any watch queries containing an entity that has been evicted will automatically refetch if partialRefetch is on, which is the desired behavior I'd want versus having to go through all possible queries that could contain it.

[benjamn]

Shortly after the AC3.0 release, we are hoping to introduce some sort of field policy configuration to specify what happens when references contained by a field no longer point anywhere.

For example, the policy could control whether the field itself gets removed, or what happens to lists that contain dangling references. I think the default behavior will continue to be leaving the dangling references untouched, but you could opt into fancier behavior where it's useful.

It's definitely tempting to do this work as part of cache.gc, since garbage collection already has to examine all the reachable references in the cache, and it's reasonable for cache.gc to follow cache.evict.

Note: with #6365, I am hoping to establish a policy that dangling references behave as much as possible like references to empty objects, so you won't see these Dangling reference ... errors anymore, but just the normal Can't find field 'name' on … errors. I'd be happy to hear any thoughts/experiences you have regarding that policy.

[jebonfig]

Note: with #6365, I am hoping to establish a policy that dangling references behave as much as possible like references to empty objects, so you won't see these Dangling reference ... errors anymore, but just the normal Can't find field 'name' on … errors. I'd be happy to hear any thoughts/experiences you have regarding that policy.

@benjamn So with these changes a cache-only query now returns an empty object {} on a cache miss? Did you all consider returning null or undefined instead of the empty object? Feels like that would be easier to handle.

A simple example being a CarsQuery using cache-and-network which fetches the entire list of cars, and a CarQuery using cache-only which gets a single car by it's id (via Query typePolicy). On page load, the CarQuery component will render _before_ the component with CarsQuery has finished loading from the server, but once it does load, the CarQuery will automatically pick up the individual item from the cache. In code that would look like:

const { 
    data: {
        car
    } = { car: null }
} = useQuery<CarData, CarVars>(CarQuery, {
    variables: {
        id: car_id!
    },
    fetchPolicy: 'cache-only'
});

if (car) {
    // render car details markup
}

//versus a more verbose check of checking for an empty object

if (car && Object.keys(car).length === 0) {
    // render car details markup    
}

[jitensuthar]

Note: with #6365, I am hoping to establish a policy that dangling references behave as much as possible like references to empty objects, so you won't see these Dangling reference ... errors anymore, but just the normal Can't find field 'name' on … errors. I'd be happy to hear any thoughts/experiences you have regarding that policy.

@benjamn So with these changes a cache-only query now returns an empty object {} on a cache miss? Did you all consider returning null or undefined instead of the empty object? Feels like that would be easier to handle.

A simple example being a CarsQuery using cache-and-network which fetches the entire list of cars, and a CarQuery using cache-only which gets a single car by it's id (via Query typePolicy). On page load, the CarQuery component will render _before_ the component with CarsQuery has finished loading from the server, but once it does load, the CarQuery will automatically pick up the individual item from the cache. In code that would look like:

@benjamn Haivng policies to affect how dangling references are handled sounds promising (and including it also as part of GC makes sense to me).

I agree with the above though in preferring cache misses come back with null or undefined fields instead of empty objects (also in returnPartialData cases) as the default behavior. It's a lot less awkward and more natural to null-test vs. testing keys to see if a field or response contains an empty object. Curious about arguments in favor of empty objects over null or undefined.

[dodas]

I agree with the above though in preferring cache misses come back with null or undefined fields instead of empty objects (also in returnPartialData cases) as the default behavior. It's a lot less awkward and more natural to null-test vs. testing keys to see if a field or response contains an empty object. Curious about arguments in favor of empty objects over null or undefined.

I think this change has been reverted and dangling references are treated the same way they were before - throwing error.
See https://github.com/apollographql/apollo-client/commit/5e74226077b88237d1f8e2cf898cf97e8ae1250e.

[jitensuthar]

I agree with the above though in preferring cache misses come back with null or undefined fields instead of empty objects (also in returnPartialData cases) as the default behavior. It's a lot less awkward and more natural to null-test vs. testing keys to see if a field or response contains an empty object. Curious about arguments in favor of empty objects over null or undefined.

I think this change has been reverted and dangling references are treated the same way they were before - throwing error.
See 5e74226.

I'm still getting back an empty object when I have returnPartialData flag set on a useQuery hook where one of the fields holds a dangling reference (resulting from an earlier cache eviction).

I was only null-testing that particular field so it fell through the cracks, which led me here. I am proposing returning null or undefined on fields that hold dangling references in a partial cache result as well (this is as of 3.0.0-rc.0)

[benjamn]

Quick updates: Yes, #6365 was reverted—sorry for that distraction. However, please have a look at #6454 (which implements https://github.com/apollographql/apollo-client/pull/6425#issuecomment-642200353) for a more automatic (and I think pretty elegant!) solution to the problem of dangling references in lists. Avoiding manual updates after eviction is an area of ongoing investigation and improvement, but I believe/hope these changes make a meaningful difference.

[benjamn]

Closing because the original example provided by @danReynolds should now work without any manual effort (the list is automatically filtered using the canRead helper, so there's no missing field error). Please open a new issue if you've got a different example you'd like us to investigate!