Compatibility mode for cache in v3 for large migration

Client SDKs
,
1

We are migrating a large monorepo to @apollo/client v3 and have around a thousand failing tests. Most of these we believe are due to the cache changes from v2 to v3. We realize that the preferred approach of course is to update all tests and mocks to work properly with v3’s API, but for now this is untenable given the number of failures.

The docs mention that dataIdFromObject is meant to ease the transition from v2 to v3, but as far as we can tell we don’t have what we need to actually shim v2’s default formatting. From the v2 docs:

If an object doesn’t specify a __typename or one of id or _id, InMemoryCache falls back to using the object’s path within its associated query (e.g., ROOT_QUERY.allPeople.0 for the first record returned for an allPeople root query).

We can’t guarantee that we have __typename (or id) available, and dataIdFromObject does not get passed in the associated query.

Is there any way to enable old-style fallback ID generation at the global level, at least in tests, in a way that would allow us to incrementally start adopting the new cache?

1 Like
2

Hey @jpm4 :wave:

Unfortunately from my findings looking through the code and trying some stuff, it doesn’t look like there is any way to get that fallback generation behavior in v3 because there is no path information available to that dataIdFromObject function in v3.

I’m curious what you’re asserting on in your tests causing them to break? It looks like if __typename is missing causing that dataIdFromObject function to return undefined, the default behavior is to treat that record as non-normalized and nest that data in the parent. My guess is that this behavior replaced the fallback cache ID in v2. It’s been a long time since I’ve used v2 so I don’t remember how the cache data is structured in that version (and I’d rather avoid spinning up a new repo if possible :sweat_smile:).

Any chance you could provide some more information on how your tests are failing? I’m hoping I can provide some guidance for you with that information.

3

Thanks @jerelmiller!

A minimal repro for many of the failures remains elusive; we’ll commonly see No more mocked responses for the query in the console output but have been unable to distill them down to something minimal. In many cases it’s likely we have improperly written tests (for example incorrect/incomplete mocks, or tests relying on the old cache behavior in a way that they should not be), which we don’t want to block the whole migration on.

A couple of specific things we’ve discovered (I can break these each out into separate discussions if you’d prefer):

  • Immutable cache results - some tests now fail because of code directly modifying the response data. What we’re confused about is that this does not appear to apply to network results, only cache results? This inconsistency seems problematic and could easily be missed depending on test cache settings.
  • Invariants (and perhaps other error types?) just console.error’ing and no longer showing up on the error object. We have tests that relied (again, perhaps improperly) on the old behavior. This led us to discovering another potential inconsistency. How are we meant to check for error/loading/data? We have code that does things such as:
const { loading, error, data } = useQuery<GetUser, GetUserVariables>(GET_USER_QUERY, {
    variables: { UserId: userId },
});

if (error) {
    throw error;
}

if (loading || !data) {
    return <LoadingComponent />;
}
  • …and then sometimes additional checks for all the properties of data later in the code. This has been done due to some combination of trying to make tests happy and TypeScript errors. In the official docs, you don’t do those extra data checks, and in theory, if loading and error are both falsey, data should be in the same shape as the gql query - that’s part of the graphql/apollo spec, right? Is there a limitation of TS here, do we have incorrect types (our type generation may be outdated), or is this all expected?
4

we’ll commonly see No more mocked responses for the query in the console output but have been unable to distill them down to something minimal

Usually this happens as a result of __typename being omitted in the test (e.g. by using addTypename={false} in MockedProvider). Any chance you could post a small snippet of one of these failing tests? I might be able to spot something there, but not sure. Its at least a clue that the query is being transformed by the Apollo Client instance in a way your test didn’t expect.

some tests now fail because of code directly modifying the response data. What we’re confused about is that this does not appear to apply to network results, only cache results?

To be honest, I’m not sure the historical reason for allowing that. Perhaps its an oversight? This one unfortunately predates my time on the team, so I’m not sure why the network request would be mutable. I know we do this for the cache because its really easy to introduce bugs this way. Mutable updates to the cache aren’t broadcasted, so its easy to get into weird inconsistencies. I agree though, its odd we don’t do this for the network response as well.

We’d strongly recommend moving away from mutability if you can in areas that the client allows you to. We do a lot of internal caching and work that relies on referential equality, so mutating values could introduce weird side effects. Totally understand this is a difficult ask and a big effort, so for now I’d just say that areas where you’re mutating data, pay attention to make sure there aren’t weird bugs as a result of it.

Invariants (and perhaps other error types?) just console.error’ing and no longer showing up on the error object

Unfortunately this is another one that predates my time on the team. My best guess is that some of the invariants were moved to warnings 1) due to feedback and 2) because the client could better handle instances where cache writes were incomplete. The client will execute a network fetch in the case that a missing field is encountered to ensure it can fulfill the query in that case.

How are we meant to check for error/loading/data

Your example looks fine to me. Are there cases where you’re seeing something unexpected such as loading: false and no data? Unless you’re using an errorPolicy of ignore, this would seem odd to me.

This has been done due to some combination of trying to make tests happy and TypeScript errors

Perhaps a code sample would be useful? I do know that unfortunately our TS types for useQuery are a tad lacking in v3. We don’t use unions on the result, so the TS type will show TData | undefined even after you’ve checked error and loading. This is something we have on the roadmap that we’d like to address soon so that data is more akin to what you’d see at runtime, which is that it should never be undefined if loading is false and you don’t have errors (unless you modify the errorPolicy to ignore).

Does this sound like the issue your’e seeing?