Skip to content

Commit

Permalink
Improve keyFields error behavior when primary key fields are missing (
Browse files Browse the repository at this point in the history
#8679)

* Make `cache.identify` return undefined instead of throwing, to help
  with `cache.identify`-related cases of issue #6673.

* Improve error message when `computeKeyFieldsObject` throws:
  #6673 (comment)
  • Loading branch information
benjamn authored Aug 26, 2021
1 parent 0f6e613 commit 2710481
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
```
[@benjamn](https://github.com/benjamn) in [#8678](https://github.com/apollographql/apollo-client/pull/8678)

- Ensure `cache.identify` never throws when primary key fields are missing, and include the source object in the error message when `keyFields` processing fails. <br/>
[@benjamn](https://github.com/benjamn) in [#8679](https://github.com/apollographql/apollo-client/pull/8679)

### React Refactoring

#### Bug Fixes (due to [@brainkim](https://github.com/brainkim) in [#8596](https://github.com/apollographql/apollo-client/pull/8596)):
Expand Down
18 changes: 15 additions & 3 deletions src/cache/inmemory/__tests__/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1772,9 +1772,21 @@ describe('EntityStore', () => {
c: 3,
})).toBe('ABCs:{"b":2,"a":1,"c":3}');

expect(() => cache.identify(ABCs)).toThrowError(
"Missing field 'b' while computing key fields",
);
{ // TODO Extact this to a helper function.
const consoleWarnSpy = jest.spyOn(console, "warn");
consoleWarnSpy.mockImplementation(() => {});
try {
expect(cache.identify(ABCs)).toBeUndefined();
expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
expect(consoleWarnSpy).toHaveBeenCalledWith(
new Error(`Missing field 'b' while extracting keyFields from ${
JSON.stringify(ABCs)
}`),
);
} finally {
consoleWarnSpy.mockRestore();
}
}

expect(cache.readFragment({
id: cache.identify({
Expand Down
6 changes: 5 additions & 1 deletion src/cache/inmemory/__tests__/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,11 @@ describe("type policies", function () {
book: theInformationBookData,
},
});
}).toThrowError("Missing field 'year' while computing key fields");
}).toThrowError(
`Missing field 'year' while extracting keyFields from ${JSON.stringify(
theInformationBookData
)}`,
);
});

it("does not clobber previous keyFields with undefined", function () {
Expand Down
9 changes: 7 additions & 2 deletions src/cache/inmemory/inMemoryCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import './fixPolyfills';
import { DocumentNode } from 'graphql';
import { OptimisticWrapperFunction, wrap } from 'optimism';
import { equal } from '@wry/equality';
import { invariant } from 'ts-invariant';

import { ApolloCache } from '../core/cache';
import { Cache } from '../core/types/Cache';
Expand Down Expand Up @@ -334,8 +335,12 @@ export class InMemoryCache extends ApolloCache<NormalizedCacheObject> {
// sure that none of the primary key fields have been renamed by aliasing.
// If you pass a Reference object, its __ref ID string will be returned.
public identify(object: StoreObject | Reference): string | undefined {
return isReference(object) ? object.__ref :
this.policies.identify(object)[0];
if (isReference(object)) return object.__ref;
try {
return this.policies.identify(object)[0];
} catch (e) {
invariant.warn(e);
}
}

public evict(options: Cache.EvictOptions): boolean {
Expand Down
4 changes: 3 additions & 1 deletion src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,9 @@ function computeKeyFieldsObject(
const responseKey = aliases && aliases[s] || s;
invariant(
hasOwn.call(response, responseKey),
`Missing field '${responseKey}' while computing key fields`,
`Missing field '${responseKey}' while extracting keyFields from ${
JSON.stringify(response)
}`,
);
keyObj[lastActualKey = s] = response[lastResponseKey = responseKey];
}
Expand Down

0 comments on commit 2710481

Please sign in to comment.