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

Ensure InMemoryCache#read{,Query,Fragment} always return T | null. #7098

Merged
merged 5 commits into from
Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
- In addition to the `result.data` property, `useQuery` and `useLazyQuery` will now provide a `result.previousData` property, which can be useful when a network request is pending and `result.data` is undefined, since `result.previousData` can be rendered instead of rendering an empty/loading state. <br/>
[@hwillson](https://github.com/hwillson) in [#7082](https://github.com/apollographql/apollo-client/pull/7082)

- Ensure `cache.readQuery` and `cache.readFragment` always return `TData | null`, instead of throwing an exception when missing fields are encountered. <br/>
[@benjamn](https://github.com/benjamn) in [#7098](https://github.com/apollographql/apollo-client/pull/7098)

## Apollo Client 3.2.1

## Bug Fixes
Expand Down
13 changes: 6 additions & 7 deletions docs/source/caching/cache-interaction.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ All code samples below assume that you have initialized an instance of `ApolloC

The `readQuery` method enables you to run a GraphQL query directly on your cache.

* If your cache contains all of the data necessary to fulfill a specified query, `readQuery` returns a data object in the shape of that query, just like a GraphQL server does.
* If your cache contains all of the data necessary to fulfill the specified query, `readQuery` returns a data object in the shape of that query, just like a GraphQL server does.

* If your cache _doesn't_ contain all of the data necessary to fulfill a specified query, `readQuery` throws an error. It _never_ attempts to fetch data from a remote server.
* If your cache does not contain all of the data necessary to fulfill the specified query, `readQuery` returns `null`, without attempting to fetch data from a remote server.

> Prior to Apollo Client 3.3, `readQuery` would throw `MissingFieldError` exceptions to report missing fields. Beginning with Apollo Client 3.3, `readQuery` always returns `null` to indicate fields were missing.

Pass `readQuery` a GraphQL query string like so:

Expand Down Expand Up @@ -81,12 +83,9 @@ const todo = client.readFragment({

The first argument, `id`, is the value of the unique identifier for the object you want to read from the cache. By default, this is the value of the object's `id` field, but you can [customize this behavior](./cache-configuration/#generating-unique-identifiers).

In the example above:
In the example above, `readFragment` returns `null` if no `Todo` object with an `id` of `5` exists in the cache, or if the object exists but is missing the `text` or `completed` fields.

* If a `Todo` object with an `id` of `5` is _not_ in the cache,
`readFragment` returns `null`.
* If the `Todo` object _is_ in the cache but it's
missing either a `text` or `completed` field, `readFragment` throws an error.
> Prior to Apollo Client 3.3, `readFragment` would throw `MissingFieldError` exceptions to report missing fields, and return `null` only when reading a fragment from a nonexistent ID. Beginning with Apollo Client 3.3, `readFragment` always returns `null` to indicate insufficient data (missing ID or missing fields), instead of throwing a `MissingFieldError`.

## `writeQuery` and `writeFragment`

Expand Down
10 changes: 6 additions & 4 deletions src/cache/core/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,14 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
* @param optimistic
*/
public readQuery<QueryType, TVariables = any>(
options: DataProxy.Query<TVariables, QueryType>,
optimistic: boolean = false,
options: Cache.ReadQueryOptions<QueryType, TVariables>,
optimistic = !!options.optimistic,
): QueryType | null {
return this.read({
rootId: options.id || 'ROOT_QUERY',
query: options.query,
variables: options.variables,
returnPartialData: options.returnPartialData,
optimistic,
});
}
Expand All @@ -120,13 +121,14 @@ export abstract class ApolloCache<TSerialized> implements DataProxy {
private getFragmentDoc = wrap(getFragmentQueryDocument);

public readFragment<FragmentType, TVariables = any>(
options: DataProxy.Fragment<TVariables, FragmentType>,
optimistic: boolean = false,
options: Cache.ReadFragmentOptions<FragmentType, TVariables>,
optimistic = !!options.optimistic,
): FragmentType | null {
return this.read({
query: this.getFragmentDoc(options.fragment, options.fragmentName),
variables: options.variables,
rootId: options.id,
returnPartialData: options.returnPartialData,
optimistic,
});
}
Expand Down
7 changes: 6 additions & 1 deletion src/cache/core/types/Cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export namespace Cache {
rootId?: string;
previousResult?: any;
optimistic: boolean;
returnPartialData?: boolean;
}

export interface WriteOptions<TResult = any, TVariables = any>
Expand All @@ -19,7 +20,9 @@ export namespace Cache {
}

export interface DiffOptions extends ReadOptions {
returnPartialData?: boolean;
// The DiffOptions interface is currently just an alias for
// ReadOptions, though DiffOptions used to be responsible for
// declaring the returnPartialData option.
}

export interface WatchOptions extends ReadOptions {
Expand All @@ -42,6 +45,8 @@ export namespace Cache {
}

export import DiffResult = DataProxy.DiffResult;
export import ReadQueryOptions = DataProxy.ReadQueryOptions;
export import ReadFragmentOptions = DataProxy.ReadFragmentOptions;
export import WriteQueryOptions = DataProxy.WriteQueryOptions;
export import WriteFragmentOptions = DataProxy.WriteFragmentOptions;
export import Fragment = DataProxy.Fragment;
Expand Down
36 changes: 33 additions & 3 deletions src/cache/core/types/DataProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export namespace DataProxy {
/**
* The root id to be used. Defaults to "ROOT_QUERY", which is the ID of the
* root query object. This property makes writeQuery capable of writing data
* to any object in the cache, which renders writeFragment mostly useless.
* to any object in the cache.
*/
id?: string;
}
Expand Down Expand Up @@ -54,6 +54,36 @@ export namespace DataProxy {
variables?: TVariables;
}

export interface ReadQueryOptions<TData, TVariables>
extends Query<TVariables, TData> {
/**
* Whether to return incomplete data rather than null.
* Defaults to false.
*/
returnPartialData?: boolean;
/**
* Whether to read from optimistic or non-optimistic cache data. If
* this named option is provided, the optimistic parameter of the
* readQuery method can be omitted. Defaults to false.
*/
optimistic?: boolean;
}

export interface ReadFragmentOptions<TData, TVariables>
extends Fragment<TVariables, TData> {
/**
* Whether to return incomplete data rather than null.
* Defaults to false.
*/
returnPartialData?: boolean;
/**
* Whether to read from optimistic or non-optimistic cache data. If
* this named option is provided, the optimistic parameter of the
* readQuery method can be omitted. Defaults to false.
*/
optimistic?: boolean;
}

export interface WriteQueryOptions<TData, TVariables>
extends Query<TVariables, TData> {
/**
Expand Down Expand Up @@ -97,7 +127,7 @@ export interface DataProxy {
* Reads a GraphQL query from the root query id.
*/
readQuery<QueryType, TVariables = any>(
options: DataProxy.Query<TVariables, QueryType>,
options: DataProxy.ReadQueryOptions<QueryType, TVariables>,
optimistic?: boolean,
): QueryType | null;

Expand All @@ -107,7 +137,7 @@ export interface DataProxy {
* provided to select the correct fragment.
*/
readFragment<FragmentType, TVariables = any>(
options: DataProxy.Fragment<TVariables, FragmentType>,
options: DataProxy.ReadFragmentOptions<FragmentType, TVariables>,
optimistic?: boolean,
): FragmentType | null;

Expand Down
24 changes: 20 additions & 4 deletions src/cache/inmemory/__tests__/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1588,13 +1588,29 @@ describe('EntityStore', () => {
},
});

expect(() => cache.readQuery({
function diff(query: DocumentNode) {
return cache.diff({
query,
optimistic: true,
returnPartialData: false,
});
}

expect(cache.readQuery({
query: queryWithAliases,
})).toThrow(/Dangling reference to missing ABCs:.* object/);
})).toBe(null);

expect(() => cache.readQuery({
expect(() => diff(queryWithAliases)).toThrow(
/Dangling reference to missing ABCs:.* object/,
);

expect(cache.readQuery({
query: queryWithoutAliases,
})).toThrow(/Dangling reference to missing ABCs:.* object/);
})).toBe(null);

expect(() => diff(queryWithoutAliases)).toThrow(
/Dangling reference to missing ABCs:.* object/,
);
});

it("gracefully handles eviction amid optimistic updates", () => {
Expand Down
51 changes: 37 additions & 14 deletions src/cache/inmemory/__tests__/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2193,17 +2193,27 @@ describe("type policies", function () {

expect(secretReadAttempted).toBe(false);

expect(() => {
cache.readQuery({
query: gql`
query {
me {
secret
}
expect(cache.readQuery({
query: gql`
query {
me {
secret
}
`
});
}).toThrowError("Can't find field 'secret' ");
}
`,
})).toBe(null);

expect(() => cache.diff({
optimistic: true,
returnPartialData: false,
query: gql`
query {
me {
secret
}
}
`,
})).toThrowError("Can't find field 'secret' ");

expect(secretReadAttempted).toBe(true);
});
Expand Down Expand Up @@ -3517,6 +3527,15 @@ describe("type policies", function () {
});
}

function diff(isbn = "156858217X") {
return cache.diff({
query,
variables: { isbn },
returnPartialData: false,
optimistic: true,
});
}

expect(read()).toBe(null);

cache.writeQuery({
Expand All @@ -3532,8 +3551,10 @@ describe("type policies", function () {
},
});

expect(read).toThrow(
/Dangling reference to missing Book:{"isbn":"156858217X"} object/
expect(read()).toBe(null);

expect(diff).toThrow(
/Dangling reference to missing Book:{"isbn":"156858217X"} object/,
);

const stealThisData = {
Expand Down Expand Up @@ -3664,11 +3685,13 @@ describe("type policies", function () {
},
});

expect(() => read("0393354326")).toThrow(
expect(read("0393354326")).toBe(null);
expect(() => diff("0393354326")).toThrow(
/Dangling reference to missing Book:{"isbn":"0393354326"} object/
);

expect(() => read("156858217X")).toThrow(
expect(read("156858217X")).toBe(null);
expect(() => diff("156858217X")).toThrow(
/Dangling reference to missing Book:{"isbn":"156858217X"} object/
);
});
Expand Down
79 changes: 79 additions & 0 deletions src/cache/inmemory/__tests__/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,85 @@ describe('reading from the store', () => {
}).toThrowError(/Can't find field 'missingField' on ROOT_QUERY object/);
});

it('readQuery supports returnPartialData', () => {
const cache = new InMemoryCache;
const aQuery = gql`query { a }`;
const bQuery = gql`query { b }`;
const abQuery = gql`query { a b }`;

cache.writeQuery({
query: aQuery,
data: { a: 123 },
});

expect(cache.readQuery({ query: bQuery })).toBe(null);
expect(cache.readQuery({ query: abQuery })).toBe(null);

expect(cache.readQuery({
query: bQuery,
returnPartialData: true,
})).toEqual({});

expect(cache.readQuery({
query: abQuery,
returnPartialData: true,
})).toEqual({ a: 123 });
});

it('readFragment supports returnPartialData', () => {
const cache = new InMemoryCache;
const id = cache.identify({
__typename: "ABObject",
id: 321,
});

const aFragment = gql`fragment AFragment on ABObject { a }`;
const bFragment = gql`fragment BFragment on ABObject { b }`;
const abFragment = gql`fragment ABFragment on ABObject { a b }`;

expect(cache.readFragment({ id, fragment: aFragment })).toBe(null);
expect(cache.readFragment({ id, fragment: bFragment })).toBe(null);
expect(cache.readFragment({ id, fragment: abFragment })).toBe(null);

const ref = cache.writeFragment({
id,
fragment: aFragment,
data: {
__typename: "ABObject",
a: 123,
},
});
expect(isReference(ref)).toBe(true);
expect(ref!.__ref).toBe(id);

expect(cache.readFragment({
id,
fragment: bFragment,
})).toBe(null);

expect(cache.readFragment({
id,
fragment: abFragment,
})).toBe(null);

expect(cache.readFragment({
id,
fragment: bFragment,
returnPartialData: true,
})).toEqual({
__typename: "ABObject",
});

expect(cache.readFragment({
id,
fragment: abFragment,
returnPartialData: true,
})).toEqual({
__typename: "ABObject",
a: 123,
});
});

it('distinguishes between missing @client and non-@client fields', () => {
const query = gql`
query {
Expand Down
Loading