Apollo-feature-requests: Return null from readQuery when data is not in store

[MaxDesiatov]

How to reproduce the issue:

const data = proxy.readQuery({query: gql`{ notInTheStore }`})

Intended outcome:

Returns null

Actual outcome:

Throws an error: ERROR Error: Uncaught (in promise): Error: Can't find field notInTheStore on object (ROOT_QUERY) undefined.

Migrated from:
apollographql/apollo-client#2007
apollographql/apollo-client#1542

[bbugh]

Longer discussion of this on a closed task (it is still a problem) https://github.com/apollographql/apollo-client/issues/1542

[lorensr]

Thanks @bbugh. Updated title & description—it looks like returning null is the favored behavior.

I'm for labeling help-wanted

[bbugh]

The consensus does seem to be to return null, but I think perhaps undefined is more correct for this use case. It's reasonable to expect that a valid readQuery would return a null value if the query is found and the value is actually null. I expect this is why the throw is there in the first place. Thus, I think the return value when a query isn't found should be undefined. Here's some silly code to demonstrate:

const query = gql`{ thing { name } }`
const data = { thing: { name: null } }

// Write a null value to the cache
store.writeQuery({ query, data })

// found query on readQuery, so it should return the value
store.readQuery(query)
=> null  // this is *actually* null

// proposal for null: doesn't make sense to return null because the query doesn't exist
// no way to tell if this is actually a real value OR the query failed
store.readQuery(gql`{ missingQuery { id } }`)
=> null  // this isn't null, it's missing

// proposal for undefined: query doesn't exist so it's not defined
store.readQuery(gql`{ missingQuery { id } }`)
=> undefined // correct - it's not actually defined

[nicky-lenaers]

Despite this being quite an annoying bug that forces us into catching the error returned from readQuery, I've created a minimal workaround to ease the pain and centrally catch the error:

import { Injectable } from '@angular/core';
import { DataProxy } from 'apollo-cache';

@Injectable({
  providedIn: 'root'
})
export class GraphQLService {

  public readQuery<T>(proxy: DataProxy, query: any): T | undefined {
    try {
      const data = proxy.readQuery<T>({ query });
      return data;
    } catch {
      return undefined;
    }
  }
}

You can consume this service anywhere and use it to "safely" use readQuery. _For now_.

[mcdougal]

I have also been using a workaround (albeit a pretty ugly one). The code below has allowed me to call readQuery the normal way but it will trap for the error and return undefined. My hope is if the behavior suggested in this issue ever makes it in to Apollo I can just remove these handful of lines without touching the rest of the codebase.

  const cache = new InMemoryCache();

  // TODO: Monkey-patching in a fix for an open issue suggesting that
  // `readQuery` should return null or undefined if the query is not yet in the
  // cache: https://github.com/apollographql/apollo-feature-requests/issues/1
  cache.originalReadQuery = cache.readQuery;
  cache.readQuery = (...args) => {
    try {
      return cache.originalReadQuery(...args);
    } catch (err) {
      return undefined;
    }
  };

  return new ApolloClient({
    link,
    cache,
  });

[seanharr11]

I'd like to contend that this is a bug and not a feature-request.

The type definition of readQuery suggests that the method can return null, which makes it even more unexpected that we get an exception. If I query a resource in a datastore (i.e. SELECT * FROM foo), by default I get an empty collection back, not a DatabaseError.

Further, if the official apollo docs suggest that the update() method is the:

"...recommended way of updating the cache after a query"

...then we really should have expected, intuitive & tight behavior here. This bug opens up a whole new class of runtime errors that won't be found until production.

These errors occur quite frequently as new features are rolled out in production (_lots of empty collections throughout the app's state!_). When a user creates a new resource in a collection, but has never "fetched that collection via an apollo query", when we try to readQuery() we get an exception & app crash.

This feels like a good candidate to be supported in v3.0. It will break the interface of those who rely on the exception to catch this error, which can be justified in a major release.

[heymanhn]

@mcdougal thanks for that monkey-patching solution you suggested. Seems most prudent and the right place to catch the errors or invariant violations.

+1 on having the Apollo team address this in v3.0.

[eltonio450]

this is clearly a non expected behavior (and very difficult to discover, it happens only in very particular cases)

I am not sure to understand in what circumstances readQuery returns null

[tuff]

This is a bug and not a feature request.

[heymanhn]

Agreed. Where is the right place to file this then?

[dylanwulf]

Actually this is documented behavior, so it's not a bug.

cache.readQuery will throw an error if the data isn't in the cache, so we need to provide an initial state
https://www.apollographql.com/docs/react/data/local-state/#writequery-and-readquery

[heymanhn]

The reason this issue is here and still open is because many of us do consider this a bug, and are asking for Apollo to reconsider the current documented behavior.

[jannes-io]

Type for readQuery already has null, so anyone using TypeScript should have backwards compatibility covered.

readQuery<QueryType, TVariables = any>(
    options: DataProxy.Query<TVariables>,
    optimistic?: boolean,
): QueryType | null;

Any update on this feature/bug?

Currently we have this in code:

  let result: ListQueryResult | null = null;
  try {
    result = cache.readQuery<ListQueryResult>({ query: list });
  } catch (e) {
    // https://github.com/apollographql/apollo-feature-requests/issues/1
  }
  const newItem = data?.createItem?.item;

  if (result !== null && newItem !== undefined) {
    cache.writeQuery({
      query: list,
      data: { items: result.items.concat([newItem]) },
    });
  }

Which is getting quite ridicules

[impowski]

Any ETA on this one?

[dylanwulf]

@impowski Check out the commit message here: https://github.com/apollographql/apollo-client/commit/6a4e4608bd0747f691e23942eacbb6545007d24f

Looks like it'll be included in v3

[impowski]

@dylanwulf oh, I’ve seen it but didn’t read the last part. Thanks.

[geiszla]

v3 is already out, but I'm getting errors when attempting to query non-existing fields. Is there any news on this issue?

[vfonic]

I'm just in the process of upgrading to v3 and this is one of the issues I'm concerned with the most. Any updates?

[LydiaF]

If it helps, I was only using readQuery to then use writeQuery with modified data, but I learned the hard way that cache.modify is the better way to go. So I have just avoided this issue entirely :)

[JesseZomer]

Any updates on this?

[benjamn]

@JesseZomer + others: it's finally happening! https://github.com/apollographql/apollo-client/pull/7098

Fine print: we would very much appreciate everyone's help validating these changes once they are released in a beta version of Apollo Client 3.3 (see #7015). If this turns out to be a major breaking change (🤞 ), it may have to wait for AC4, but we are hopeful that it will not be very disruptive, since it merely turns exceptions into null return values, and null was already a possible return value.

Thanks to everyone (especially @MaxDesiatov and @lorensr, originally) for voicing your support for this change over the past 2+ years! Hopefully it sticks this time!