apollo-client: Under-fetching fields of new objects in mutations silently breaks mounted Queries

Intended outcome: When a mounted query can not resolve its data from the cache, as in the case following an under-fetching mutation, (linked example app below), I would expect it to re-fetch.

Actual outcome: The query does not re-fetch, but instead renders with data: {}. In a real application, the component will likely attempt to access properties of undefined, causing a crash.

How to reproduce the issue: Error template app here: https://github.com/stevehollaar/react-apollo-error-template/tree/mutate-bug apollo-bug There are 2 mutation buttons, each which create a new Pet object and assign it to a random person.

Version

  • apollo-client@2.2.8

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 15
  • Comments: 22 (10 by maintainers)

Most upvoted comments

@stefanmaric I have a fix for this. Will open a PR soon

Edit: Fix is up in https://github.com/apollographql/react-apollo/pull/2003

re @pleunv: I’ve published v2.1.11 version of this package with the patch applied to @steelbrain/react-apollo, We’re using it in our app in production

The first reports of this issue date from almost a year ago, there’s a repro (#3267), there’s an outstanding PR, but other than that there’s very little to no info and no acknowledgement that this is being looked at, or that additional help is needed. I fully understand that this is an open-source project and I absolutely massively appreciate the efforts of the people involved, but the lack of communication on this (in my opinion, very severe) issue is slowly getting a bit frustrating. It feels like efforts are being focused elsewhere, which is fine of course, but I guess it would be nice to know if that’s the case. Apollo-client and react-apollo are a core component of my (and many other people’s) apps but I’m getting a little bit uncomfortable using it again for any new ones.

To workaround this bug unitl it’s fixed, I’ve used this ugly hack and has worked so far:

  shouldComponentUpdate ({ data }) {
    if (data && !data.loading && !data.error && !data.dataYouWereExpecting) {
      data.refetch()
      return false
    }

    return true
  }

Simply refetch the query if it became broken and prevent buggy renders and flashes while it refetches.

I’ve had similar observations when upgrading from 2.0 to 2.1.

When 2 queries that fetch the same node but with different fields run in sequence, the second query initially renders a partial node but with missing fields, instead of presenting the previous behavior of rendering node: undefined while in transit.

For example, when you have the following 2 queries:

query Profile($id: GUID!) {
  profile(id: $id) {
    id
    firstName
    lastName
  }
}

query ProfileWithFields($id: GUID!) {
  profile(id: $id) {
    id
    firstName
    lastName
    fields {
      name
      value
    }
  }
}

The second query will initially be rendered with

profile: {
  id: 123,
  firstName: 'John',
  lastName: 'Doe', 
  fields: undefined
}

whereas before it would initially have rendered with profile: undefined while waiting for the network response.

In our case this is a little bit problematic since we’re not checking loading states but rather whether profile !== undefined, and are expecting Profile.fields to be an array instead of undefined as soon as the data gets passed down. If this is a conscious design change this would require our mappers to either look at loading states instead, or our components to be able to deal with partial data, which, as you can suspect, is a bit of a if (x !== undefined) nightmare.

I haven’t had time yet to dig into or set up a repro for this and reverted back to 2.0 for now but I have a suspicion it might be related to the subject of this issue.

Squeezed in some preliminary testing with @steelbrain’s version and I’m not able to reproduce the underfetching issue! Will keep using it for the next few days because I think some more testing will be needed, and let you guys know if we do run into any problems. I’m really glad this is moving forward! I know you guys are juggling a lot of different projects and keeping a business running at the same time, and I really do appreciate all the time you put into this.

Out of curiousity (mainly since you already mention 3.0): is there any public roadmap available, or is this still being discussed behind closed doors? 😃

Hey, as our issue with optimisticResponses might be very much related, I am posting this here. We reproduced our issue https://github.com/whootor/react-apollo-error-template/tree/optimist.

Example 1

Example 1

  1. An initial component with an underfetched query is mounted, fetched and rendered
  2. A second component with a fully fetched query is mounted delayed
  3. Shortly after the fully fetched query is mounted, an underfetched mutation is triggered
  4. The mutation triggers an optimistic response which is stored in the cache (the initial component shows the optimistic response)
  5. The fully fetched query response comes in and updates the second component with loading=false and no data
  6. The mutation response comes in and updates both components, the second one with the actually merged data of the full query and the underfetched mutation response.

At 5. we expect one of two behaviours:

  1. the second component to not be updated at all, only when the final merged result comes in (stays in loading state)
  2. the second component is updated with a merged result of the optimistic response and the full query result

Another issue that arises in Example 3 is that the mutation response is overwritten by the fully fetched query response with old data. As the mutation was triggered after the full query was, it should have precedence before the full query and be merged on top of the full query.

We believe to have traced down the issue to the merging in the in-memory-cache, but didn’t get any further so far.