Nested objects with no ids and different selection sets returns no data

[ford-outreach]

Intended outcome:
With returnPartialData false, requesting non-matching selection sets from nested, denormalized objects (with no id), should alert me that there is data loss, and therefore complete data can never be achieved.

In this case, our User has a set of Preferences, and under preferences there are CalendarPreferences. Neither Preferences nor CalendarPreferences has their own id.

Take this very contrived query (I promise there's a realistic query with the same symptoms)

query {
  userWithPreferenceOne: user(id: 1) {
    id
    preferences {
      calendarPreferences {
        preferenceOne
      }
   }
  userWithPreferenceTwo: user(id: 1) {
    id
    preferences {
      calendarPreferences {
        preferenceTwo
      }
   }
 }
}

Apollo Client should alert me that because there is no custom merge function defined for Preferences or CalendarPreferences, and because there are no ids, it won't merge them, and therefore either userWithPreferenceOne or userWithPreferenceTwo will never have complete data.

At the very least, warnAboutDataLoss should fire,
however in a case like this where data will never resolve, an error seems appropriate.

Actual outcome:
When a query like the one above is made using the useQuery hook. The results are

error: undefined,
data: undefined,
loading: false

There are no errors or warnings logged, so there is no indication that anything failed. However the query is 'finished' and yet there is no data and no error.

With returnPartialData on, the incomplete data is returned (turning this on is undesirable, we need all the data).
Turning on partialRefetch leads to an infinite loop of trying to load the complete data and failing.

I stepped through the warnAboutDataLoss function and the problematic object (preferences, in the above example) early returns on this condition:

apollo-client/src/cache/inmemory/writeToStore.ts

Lines 362 to 365 in a975320

	if (Object.keys(existing).every(
	key => store.getFieldValue(incoming, key) !== void 0)) {
	return;
	}

Since it is only checking top level properties, it verifies that 'existing' and 'incoming' both have 'calendarPreferences' fields, but it does not catch that one only has preferenceOne and one only has preferenceTwo.

How to reproduce the issue:
I have created a reproduction here: https://github.com/ford-outreach/react-apollo-error-template
Here is the diff (apologies that it's a little noisy, prettier went to town): apollographql/react-apollo-error-template@master...ford-outreach:master

Versions
3.0.2 in our app
3.2.5 in the reproduction

[benjamn]

@ford-outreach Your analysis seems spot-on! I fully believe warnAboutDataLoss could and should have flagged this problem. We will get that fixed shortly, though I'm also happy to continue discussing the best way to fix these kinds of problems, once they've been flagged by the warning system.

[ford-outreach]

Thank you for the quick response @benjamn , that's great to hear! We added ids to Preferences and CalendarPreferences and that's fixed the immediate problem, I think a custom merge function would have also done the trick?

The warning message from warnAboutDataLoss seems super useful, I think had that gone off, it would've been pretty easy to get to the root cause

[benjamn]

Yep, the goal of warnAboutDataLoss is to get people to either define keyFields (or add an id) or write a custom merge function, though I think the first strategy is preferable, when feasible.

[ford-outreach]

That makes sense
In this case, the objects in question are sort of pseudo objects at the moment that only exist in the GraphQL layer (hence why they had no ids). Someday they'll have real things with ids backing them, in the meantime we came up with something we can use as a unique identifier for now

[vieira]

We were also bitten by this (in 3.3.0-rc.2). We did migrate from 2.x where this used to produce the expected behaviour (most of the time hehe).

As there was no warnings and the query asking for nested object without id was on a different component than the one exhibiting the problem (returning data undefined) it took a while to figure out the issue.

query GetBook {
  book(id: $id) {
    id
    category {
      # id is missing
      name
    }
    publishedAt
    numPages
  }

query GetBook {
  book(id: $id) {
    id
    reviews {
      edges {
        node {
          text
          createdAt
        }
      }
    }
    category {
      id
      name
    }
    publishedAt
    numPages
  }

The second query will return data undefined.

EDIT: Both queries were asking for the category, one of them without id.

Thanks!

[leethree]

We encountered the same issue. Agree there should be a warning about this.

We workaround this issue by using keyFields to generate IDs:
https://www.apollographql.com/docs/react/caching/cache-configuration/#customizing-identifier-generation-by-type

[migueloller]

Another solution instead of writing a custom merge function, is using merge: true for the given field as documented here

[leethree]

@migueloller I think merge: true works only with objects. In that case there should already be a warning telling users what to do.

For lists we still need to use either a custom merge function or adding ID/keyFields to the list items (as noted by #7315 (comment)). And the warning doesn't show up for some reason.

[migueloller]

@leethree, ah, thanks for pointing that out!