Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning on missing data for observable query #2262

Closed
jer-sen opened this issue Oct 6, 2017 · 11 comments
Closed

Warning on missing data for observable query #2262

jer-sen opened this issue Oct 6, 2017 · 11 comments

Comments

@jer-sen
Copy link
Contributor

jer-sen commented Oct 6, 2017

When an observed query can't be updated because data is missing in the cache, there should be a console.warn because it means that:

  1. Data are not consistent in the app
  2. There is a mistake in the choice of the queries observed or in the choice of field selected in the response to some mutations/queries

For instance, if you call a mutation to add a new product to a shopping basket with only a table of product's ids selected in the response, and if you have an observed query on the basket with product's field "weight" selected, the query won't be updated and thus the user won't see the new product.

Ideally the warning should tell which data is missing.

@jbaxleyiii
Copy link
Contributor

@Jay1337 I'm all for improving the development warnings / guidance of Apollo! Do you have a sample app which showcases this behavior?

@jer-sen
Copy link
Contributor Author

jer-sen commented Oct 11, 2017

@jbaxleyiii I don't have (time to provide) a full sample app but here are some code to make the example in my first message more concrete.

The schema could be:

type Query {
  user(id: ID!): User!
}

type User {
  id
  basket: [Product!]!
}

type Product {
  id
  name
  weight
}

type Mutation {
  addProductToBasket(user: ID!, product: ID!) User!
}

The query observed by a React component could be:

query {
  user(id: 123) {
    id
    basket {
      id
      name
      weight
    }
  }
}

The mutation triggered when the user adds a product to the basket could be:

mutation {
  addProductToBasket(user: 123, product: 456) {
    id
    basket {
      id
      name
    }
}

Since weight is not selected in basket, data will be missing to refetch the query from Apollo cache after the mutation, thus the basket React component won't get updated. A warning could easily be added in Apollo code to help the developer.

@jer-sen
Copy link
Contributor Author

jer-sen commented Oct 12, 2017

@jbaxleyiii I updated to:

apollo-cache-inmemory@0.2.0-rc.0
apollo-client@2.0.0-rc.1

And now if data is missing, the partial data are passed to the wrapped component anyway !
So I can get !this.props.data.error && !this.props.data.loading && this.props.myRequiredQueryField === undefined !

@stale
Copy link

stale bot commented Nov 2, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@jer-sen
Copy link
Contributor Author

jer-sen commented Nov 2, 2017

Hey !

@jbaxleyiii
Copy link
Contributor

@Jay1337 sorry about that! I forgot to add the label to keep this open as an area of improvement!

@stale
Copy link

stale bot commented Nov 27, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@jer-sen
Copy link
Contributor Author

jer-sen commented Nov 27, 2017

Hey !

@msimulcik
Copy link
Contributor

@jbaxleyiii any update on this? Lack of any warning caused nasty bug in our application. We are using cache-only query to get logged user from cache relying on data returned previously by login mutation. The problem was that mutation didn't return all data requested by getLoggedUserQuery which resulted in error during rendering caused by missing data in props.

I did a little bit of debugging of Apollo and ended up with adding following code to ObservableQuery.currentResult. I also had to return caught error from QueryManager.getCurrentQueryResult as it contains information about missing field.

const isLoading = isNetworkRequestInFlight(networkStatus);
if (!isLoading
    && !this.options.returnPartialData
    && partial
) {
    console.warn(error);
}

Logic behind this is that if there are no data being loaded it most likely means that partial data are caused by error in query. I'm not sure about && !this.options.returnPartialData part of condition.

I can create PR for this but I wanted to discuss it first because there's a high chance that I'm missing something.

@derek-duncan
Copy link

We're having the same issue as @msimulcik. Sometimes partial data is returned from the cache while the data is first loading. Thus, sometimes data has incomplete data, which destroys our confidence in the object structure, forcing unnecessary null-checks. It'd be great if data could remain empty when networkStatus is 1, even if partial cache data is found.

@hwillson
Copy link
Member

hwillson commented Jul 27, 2018

To help provide a more clear separation between feature requests / discussions and bugs, and to help clean up the feature request / discussion backlog, Apollo Client feature requests / discussions are now being managed under the https://github.com/apollographql/apollo-feature-requests repository.

Migrated to: apollographql/apollo-feature-requests#16

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants