diff --git a/CHANGELOG.md b/CHANGELOG.md index 5080702b183..5a67c6d4616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,9 @@ - The [`nextFetchPolicy`](https://github.com/apollographql/apollo-client/pull/6893) option for `client.watchQuery` and `useQuery` will no longer be removed from the `options` object after it has been applied, and instead will continue to be applied any time `options.fetchPolicy` is reset to another value, until/unless the `options.nextFetchPolicy` property is removed from `options`.
[@benjamn](https://github.com/benjamn) in [#8465](https://github.com/apollographql/apollo-client/pull/8465) +- Make `readField` default to reading from current object only when the `from` option/argument is actually omitted, not when `from` is passed to `readField` with an undefined value. A warning will be printed when this situation occurs.
+ [@benjamn](https://github.com/benjamn) in [#8508](https://github.com/apollographql/apollo-client/pull/8508) + ### Improvements - `InMemoryCache` now _guarantees_ that any two result objects returned by the cache (from `readQuery`, `readFragment`, etc.) will be referentially equal (`===`) if they are deeply equal. Previously, `===` equality was often achievable for results for the same query, on a best-effort basis. Now, equivalent result objects will be automatically shared among the result trees of completely different queries. This guarantee is important for taking full advantage of optimistic updates that correctly guess the final data, and for "pure" UI components that can skip re-rendering when their input data are unchanged.
diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index f43c18e7b25..90edc0545ba 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -312,9 +312,12 @@ Array [ "itAsync", "mockObservableLink", "mockSingleLink", + "stringifyForDisplay", "stripSymbols", "subscribeAndCount", "withErrorSpy", + "withLogSpy", + "withWarningSpy", ] `; diff --git a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap index d414466e566..7db3045a6a0 100644 --- a/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap +++ b/src/cache/inmemory/__tests__/__snapshots__/policies.ts.snap @@ -1389,6 +1389,29 @@ exports[`type policies field policies runs nested merge functions as well as anc } `; +exports[`type policies readField warns if explicitly passed undefined \`from\` option 1`] = ` +[MockFunction] { + "calls": Array [ + Array [ + "Undefined 'from' passed to readField with arguments [{\\"fieldName\\":\\"firstName\\",\\"from\\":}]", + ], + Array [ + "Undefined 'from' passed to readField with arguments [\\"lastName\\",]", + ], + ], + "results": Array [ + Object { + "type": "return", + "value": undefined, + }, + Object { + "type": "return", + "value": undefined, + }, + ], +} +`; + exports[`type policies support inheritance 1`] = ` Object { "Cobra:{\\"tagId\\":\\"Egypt30BC\\"}": Object { diff --git a/src/cache/inmemory/__tests__/policies.ts b/src/cache/inmemory/__tests__/policies.ts index 3fe2c070e51..6ba256d2d7e 100644 --- a/src/cache/inmemory/__tests__/policies.ts +++ b/src/cache/inmemory/__tests__/policies.ts @@ -9,7 +9,7 @@ import { MockLink } from '../../../utilities/testing/mocking/mockLink'; import subscribeAndCount from '../../../utilities/testing/subscribeAndCount'; import { itAsync } from '../../../utilities/testing/itAsync'; import { FieldPolicy, StorageType } from "../policies"; -import { withErrorSpy } from "../../../testing"; +import { withErrorSpy, withWarningSpy } from "../../../testing"; function reverse(s: string) { return s.split("").reverse().join(""); @@ -5051,6 +5051,70 @@ describe("type policies", function () { }); }); + withWarningSpy(it, "readField warns if explicitly passed undefined `from` option", function () { + const cache = new InMemoryCache({ + typePolicies: { + Query: { + fields: { + fullNameWithDefaults(_, { readField }) { + return `${ + readField({ + fieldName: "firstName", + }) + } ${ + readField("lastName") + }`; + }, + + fullNameWithVoids(_, { readField }) { + return `${ + readField({ + fieldName: "firstName", + // If options.from is explicitly passed but undefined, + // readField should not default to reading from the current + // object (see issue #8499). + from: void 0, + }) + } ${ + // Likewise for the shorthand version of readField. + readField("lastName", void 0) + }`; + }, + }, + }, + }, + }); + + const firstNameLastNameQuery = gql` + query { + firstName + lastName + } + `; + + const fullNamesQuery = gql` + query { + fullNameWithVoids + fullNameWithDefaults + } + `; + + cache.writeQuery({ + query: firstNameLastNameQuery, + data: { + firstName: "Alan", + lastName: "Turing", + }, + }); + + expect(cache.readQuery({ + query: fullNamesQuery, + })).toEqual({ + fullNameWithDefaults: "Alan Turing", + fullNameWithVoids: "undefined undefined", + }); + }); + it("can return existing object from merge function (issue #6245)", function () { const cache = new InMemoryCache({ typePolicies: { diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index f32c906ef2a..3a623aa3541 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -52,6 +52,7 @@ import { WriteContext } from './writeToStore'; // used by getStoreKeyName. This function is used when computing storeFieldName // strings (when no keyArgs has been configured for a field). import { canonicalStringify } from './object-canon'; +import { stringifyForDisplay } from '../../testing'; getStoreKeyName.setStringify(canonicalStringify); export type TypePolicies = { @@ -882,14 +883,36 @@ function makeFieldFunctionOptions( fieldNameOrOptions: string | ReadFieldOptions, from?: StoreObject | Reference, ) { - const options: ReadFieldOptions = - typeof fieldNameOrOptions === "string" ? { + let options: ReadFieldOptions; + if (typeof fieldNameOrOptions === "string") { + options = { fieldName: fieldNameOrOptions, - from, - } : { ...fieldNameOrOptions }; + // Default to objectOrReference only when no second argument was + // passed for the from parameter, not when undefined is explicitly + // passed as the second argument. + from: arguments.length > 1 ? from : objectOrReference, + }; + } else if (isNonNullObject(fieldNameOrOptions)) { + options = { ...fieldNameOrOptions }; + // Default to objectOrReference only when fieldNameOrOptions.from is + // actually omitted, rather than just undefined. + if (!hasOwn.call(fieldNameOrOptions, "from")) { + options.from = objectOrReference; + } + } else { + invariant.warn(`Unexpected readField arguments: ${ + stringifyForDisplay(Array.from(arguments)) + }`); + // The readField helper function returns undefined for any missing + // fields, so it should also return undefined if the arguments were not + // of a type we expected. + return; + } - if (void 0 === options.from) { - options.from = objectOrReference; + if (__DEV__ && options.from === void 0) { + invariant.warn(`Undefined 'from' passed to readField with arguments ${ + stringifyForDisplay(Array.from(arguments)) + }`); } if (void 0 === options.variables) { diff --git a/src/utilities/testing/index.ts b/src/utilities/testing/index.ts index 668557783e9..c0e5c1daa51 100644 --- a/src/utilities/testing/index.ts +++ b/src/utilities/testing/index.ts @@ -13,4 +13,5 @@ export { createMockClient } from './mocking/mockClient'; export { stripSymbols } from './stripSymbols'; export { default as subscribeAndCount } from './subscribeAndCount'; export { itAsync } from './itAsync'; -export { withErrorSpy } from './withErrorSpy'; +export * from './withConsoleSpy'; +export * from './stringifyForDisplay'; diff --git a/src/utilities/testing/mocking/mockLink.ts b/src/utilities/testing/mocking/mockLink.ts index 7b3a24b49ba..4812177b4b7 100644 --- a/src/utilities/testing/mocking/mockLink.ts +++ b/src/utilities/testing/mocking/mockLink.ts @@ -15,17 +15,13 @@ import { removeClientSetsFromDocument, removeConnectionDirectiveFromDocument, cloneDeep, - makeUniqueId, } from '../../../utilities'; -export type ResultFunction = () => T; +import { + stringifyForDisplay, +} from '../../../testing'; -function stringifyForDisplay(value: any): string { - const undefId = makeUniqueId("stringifyForDisplay"); - return JSON.stringify(value, (key, value) => { - return value === void 0 ? undefId : value; - }).split(JSON.stringify(undefId)).join(""); -} +export type ResultFunction = () => T; export interface MockedResponse> { request: GraphQLRequest; diff --git a/src/utilities/testing/stringifyForDisplay.ts b/src/utilities/testing/stringifyForDisplay.ts new file mode 100644 index 00000000000..4eb8a724238 --- /dev/null +++ b/src/utilities/testing/stringifyForDisplay.ts @@ -0,0 +1,8 @@ +import { makeUniqueId } from "../common/makeUniqueId"; + +export function stringifyForDisplay(value: any): string { + const undefId = makeUniqueId("stringifyForDisplay"); + return JSON.stringify(value, (key, value) => { + return value === void 0 ? undefId : value; + }).split(JSON.stringify(undefId)).join(""); +} diff --git a/src/utilities/testing/withConsoleSpy.ts b/src/utilities/testing/withConsoleSpy.ts new file mode 100644 index 00000000000..171a6eac2a5 --- /dev/null +++ b/src/utilities/testing/withConsoleSpy.ts @@ -0,0 +1,49 @@ +function wrapTestFunction( + fn: (...args: any[]) => any, + consoleMethodName: "log" | "warn" | "error", +) { + return function () { + const args = arguments; + const spy = jest.spyOn(console, consoleMethodName); + spy.mockImplementation(() => {}); + return new Promise(resolve => { + resolve(fn?.apply(this, args)); + }).finally(() => { + expect(spy).toMatchSnapshot(); + spy.mockReset(); + }); + }; +} + +export function withErrorSpy< + TArgs extends any[], + TResult, +>( + it: (...args: TArgs) => TResult, + ...args: TArgs +) { + args[1] = wrapTestFunction(args[1], "error"); + return it(...args); +} + +export function withWarningSpy< + TArgs extends any[], + TResult, +>( + it: (...args: TArgs) => TResult, + ...args: TArgs +) { + args[1] = wrapTestFunction(args[1], "warn"); + return it(...args); +} + +export function withLogSpy< + TArgs extends any[], + TResult, +>( + it: (...args: TArgs) => TResult, + ...args: TArgs +) { + args[1] = wrapTestFunction(args[1], "log"); + return it(...args); +} diff --git a/src/utilities/testing/withErrorSpy.ts b/src/utilities/testing/withErrorSpy.ts deleted file mode 100644 index 2c39e8c1f91..00000000000 --- a/src/utilities/testing/withErrorSpy.ts +++ /dev/null @@ -1,21 +0,0 @@ -export function withErrorSpy< - TArgs extends any[], - TResult, ->( - it: (...args: TArgs) => TResult, - ...args: TArgs -) { - const fn = args[1]; - args[1] = function () { - const args = arguments; - const errorSpy = jest.spyOn(console, 'error'); - errorSpy.mockImplementation(() => {}); - return new Promise(resolve => { - resolve(fn?.apply(this, args)); - }).finally(() => { - expect(errorSpy).toMatchSnapshot(); - errorSpy.mockReset(); - }); - }; - return it(...args); -}