diff --git a/.api-reports/api-report-cache.api.md b/.api-reports/api-report-cache.api.md index b539bec11c8..243e585f909 100644 --- a/.api-reports/api-report-cache.api.md +++ b/.api-reports/api-report-cache.api.md @@ -1109,9 +1109,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // // src/cache/core/cache.ts:92:7 - (ae-forgotten-export) The symbol "MaybeMasked" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:93:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:139:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/.api-reports/api-report-core.api.md b/.api-reports/api-report-core.api.md index fe19a8ee4fd..3a31996bbd3 100644 --- a/.api-reports/api-report-core.api.md +++ b/.api-reports/api-report-core.api.md @@ -2432,9 +2432,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // -// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:93:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:139:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:120:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:121:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts diff --git a/.api-reports/api-report-utilities.api.md b/.api-reports/api-report-utilities.api.md index 451fe8a5d66..32893220155 100644 --- a/.api-reports/api-report-utilities.api.md +++ b/.api-reports/api-report-utilities.api.md @@ -2792,11 +2792,11 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // // src/cache/core/types/DataProxy.ts:147:7 - (ae-forgotten-export) The symbol "MissingFieldError" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:57:3 - (ae-forgotten-export) The symbol "TypePolicy" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "FieldReadFunction" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:163:3 - (ae-forgotten-export) The symbol "FieldMergeFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:58:3 - (ae-forgotten-export) The symbol "TypePolicy" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:163:3 - (ae-forgotten-export) The symbol "FieldReadFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:164:3 - (ae-forgotten-export) The symbol "FieldMergeFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:139:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/writeToStore.ts:65:7 - (ae-forgotten-export) The symbol "MergeTree" needs to be exported by the entry point index.d.ts // src/core/LocalState.ts:71:3 - (ae-forgotten-export) The symbol "ApolloClient" needs to be exported by the entry point index.d.ts diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index e94c5e35783..0a2c8f55e31 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -3143,9 +3143,9 @@ interface WriteContext extends ReadMergeModifyContext { // Warnings were encountered during analysis: // -// src/cache/inmemory/policies.ts:92:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts -// src/cache/inmemory/policies.ts:161:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:93:3 - (ae-forgotten-export) The symbol "FragmentMap" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeySpecifier" needs to be exported by the entry point index.d.ts +// src/cache/inmemory/policies.ts:162:3 - (ae-forgotten-export) The symbol "KeyArgsFunction" needs to be exported by the entry point index.d.ts // src/cache/inmemory/types.ts:139:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:120:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:121:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts diff --git a/.changeset/early-bobcats-eat.md b/.changeset/early-bobcats-eat.md new file mode 100644 index 00000000000..64907a4bda9 --- /dev/null +++ b/.changeset/early-bobcats-eat.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Prevent field accessor warnings when using `@unmask(mode: "migrate")` on objects that are passed into `cache.identify`. diff --git a/.size-limits.json b/.size-limits.json index 1a3c2258651..0a00c26d167 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 41506, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34257 + "dist/apollo-client.min.cjs": 41516, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34296 } diff --git a/src/__tests__/dataMasking.ts b/src/__tests__/dataMasking.ts index f8bcd12ceb0..dfe2988c0f3 100644 --- a/src/__tests__/dataMasking.ts +++ b/src/__tests__/dataMasking.ts @@ -1143,6 +1143,73 @@ describe("client.watchQuery", () => { } }); + // https://github.com/apollographql/apollo-client/issues/12043 + test("does not warn when passing @unmask(mode: 'migrate') object to cache.identify", async () => { + using consoleSpy = spyOnConsole("warn"); + + type UserFieldsFragment = { + age: number; + } & { " $fragmentName"?: "UserFieldsFragment" }; + + interface Query { + currentUser: { + __typename: "User"; + id: number; + name: string; + /** @deprecated */ + age: number; + } & { " $fragmentRefs"?: { UserFieldsFragment: UserFieldsFragment } }; + } + + const query: MaskedDocumentNode = gql` + query UnmaskedQuery { + currentUser { + id + name + ...UserFields @unmask(mode: "migrate") + } + } + + fragment UserFields on User { + age + name + } + `; + + const mocks = [ + { + request: { query }, + result: { + data: { + currentUser: { + __typename: "User", + id: 1, + name: "Test User", + age: 34, + }, + }, + }, + delay: 20, + }, + ]; + + const client = new ApolloClient({ + dataMasking: true, + cache: new InMemoryCache(), + link: new MockLink(mocks), + }); + + const observable = client.watchQuery({ query }); + const stream = new ObservableStream(observable); + + const { data } = await stream.takeNext(); + + const id = client.cache.identify(data.currentUser); + + expect(consoleSpy.warn).not.toHaveBeenCalled(); + expect(id).toEqual("User:1"); + }); + test("reads fragment by passing parent object to `from`", async () => { type UserFieldsFragment = { age: number; @@ -3112,79 +3179,84 @@ describe("client.watchFragment", () => { }); }); - // FIXME: This broke with the changes in https://github.com/apollographql/apollo-client/pull/12114 - // which ensure masking works with deferred payloads. Instead of fixing with - // #12114, it will be fixed with https://github.com/apollographql/apollo-client/issues/12043 - // which will fix overagressive warnings. - test.failing( - "warns when accessing an unmasked field on a watched fragment while using @unmask with mode: 'migrate'", - async () => { - using consoleSpy = spyOnConsole("warn"); + test("warns when accessing an unmasked field on a watched fragment while using @unmask with mode: 'migrate'", async () => { + using consoleSpy = spyOnConsole("warn"); - type ProfileFieldsFragment = { - __typename: "User"; - age: number; - name: string; - } & { " $fragmentName": "UserFieldsFragment" }; + type ProfileFieldsFragment = { + __typename: "User"; + age: number; + name: string; + } & { " $fragmentName": "UserFieldsFragment" }; - type UserFieldsFragment = { - __typename: "User"; - id: number; - name: string; - /** @deprecated */ - age: number; - } & { " $fragmentName": "UserFieldsFragment" } & { - " $fragmentRefs": { ProfileFieldsFragment: ProfileFieldsFragment }; - }; + type UserFieldsFragment = { + __typename: "User"; + id: number; + name: string; + /** @deprecated */ + age: number; + } & { " $fragmentName": "UserFieldsFragment" } & { + " $fragmentRefs": { ProfileFieldsFragment: ProfileFieldsFragment }; + }; - const fragment: MaskedDocumentNode = gql` - fragment UserFields on User { - id - name - ...ProfileFields @unmask(mode: "migrate") - } + const fragment: MaskedDocumentNode = gql` + fragment UserFields on User { + id + name + ...ProfileFields @unmask(mode: "migrate") + } - fragment ProfileFields on User { - age - name - } - `; + fragment ProfileFields on User { + age + name + } + `; - const client = new ApolloClient({ - dataMasking: true, - cache: new InMemoryCache(), - }); + const client = new ApolloClient({ + dataMasking: true, + cache: new InMemoryCache(), + }); - const observable = client.watchFragment({ - fragment, - fragmentName: "UserFields", - from: { __typename: "User", id: 1 }, - }); - const stream = new ObservableStream(observable); + client.writeFragment({ + id: client.cache.identify({ __typename: "User", id: 1 }), + fragment, + fragmentName: "UserFields", + data: { + __typename: "User", + id: 1, + age: 30, + name: "Test User", + }, + }); - { - const { data } = await stream.takeNext(); - data.__typename; - data.id; - data.name; + const observable = client.watchFragment({ + fragment, + fragmentName: "UserFields", + from: { __typename: "User", id: 1 }, + }); + const stream = new ObservableStream(observable); - expect(consoleSpy.warn).not.toHaveBeenCalled(); + { + const { data } = await stream.takeNext(); + data.__typename; + data.id; + data.name; - data.age; + expect(consoleSpy.warn).not.toHaveBeenCalled(); - expect(consoleSpy.warn).toHaveBeenCalledTimes(1); - expect(consoleSpy.warn).toHaveBeenCalledWith( - "Accessing unmasked field on %s at path '%s'. This field will not be available when masking is enabled. Please read the field from the fragment instead.", - "fragment 'UserFields'", - "age" - ); + data.age; - // Ensure we only warn once - data.age; - expect(consoleSpy.warn).toHaveBeenCalledTimes(1); - } + expect(consoleSpy.warn).toHaveBeenCalledTimes(1); + expect(consoleSpy.warn).toHaveBeenCalledWith( + "Accessing unmasked field on %s at path '%s'. This field will not be available when masking is enabled. Please read the field from the fragment instead.", + "fragment 'UserFields'", + "age" + ); + + // Ensure we only warn once + data.age; + expect(consoleSpy.warn).toHaveBeenCalledTimes(1); } - ); + }); test("can lookup unmasked fragments from the fragment registry in watched fragments", async () => { const fragments = createFragmentRegistry(); diff --git a/src/cache/inmemory/policies.ts b/src/cache/inmemory/policies.ts index 3c68f35eb9d..774c167cd11 100644 --- a/src/cache/inmemory/policies.ts +++ b/src/cache/inmemory/policies.ts @@ -52,6 +52,7 @@ import { keyArgsFnFromSpecifier, keyFieldsFnFromSpecifier, } from "./key-extractor.js"; +import { disableWarningsSlot } from "../../core/masking.js"; export type TypePolicies = { [__typename: string]: TypePolicy; @@ -391,15 +392,18 @@ export class Policies { const policy = typename && this.getTypePolicy(typename); let keyFn = (policy && policy.keyFn) || this.config.dataIdFromObject; - while (keyFn) { - const specifierOrId = keyFn({ ...object, ...storeObject }, context); - if (isArray(specifierOrId)) { - keyFn = keyFieldsFnFromSpecifier(specifierOrId); - } else { - id = specifierOrId; - break; + + disableWarningsSlot.withValue(true, () => { + while (keyFn) { + const specifierOrId = keyFn({ ...object, ...storeObject }, context); + if (isArray(specifierOrId)) { + keyFn = keyFieldsFnFromSpecifier(specifierOrId); + } else { + id = specifierOrId; + break; + } } - } + }); id = id ? String(id) : void 0; return context.keyObject ? [id, context.keyObject] : [id]; diff --git a/src/core/__tests__/masking.test.ts b/src/core/__tests__/masking.test.ts index 27063e7be1d..84f5389e928 100644 --- a/src/core/__tests__/masking.test.ts +++ b/src/core/__tests__/masking.test.ts @@ -2244,7 +2244,7 @@ describe("maskFragment", () => { test("warns when accessing unmasked fields when using `@unmask` directive with mode 'migrate'", () => { using _ = spyOnConsole("warn"); - const query = gql` + const fragment = gql` fragment UnmaskedFragment on User { id name @@ -2258,18 +2258,22 @@ describe("maskFragment", () => { const data = maskFragment( deepFreeze({ - currentUser: { - __typename: "User", - id: 1, - name: "Test User", - age: 30, - }, + __typename: "User", + id: 1, + name: "Test User", + age: 30, }), - query, + fragment, new InMemoryCache(), "UnmaskedFragment" ); + data.__typename; + data.id; + data.name; + + expect(console.warn).not.toHaveBeenCalled(); + data.age; expect(console.warn).toHaveBeenCalledTimes(1); diff --git a/src/core/masking.ts b/src/core/masking.ts index 3991940a23c..a6d288b9653 100644 --- a/src/core/masking.ts +++ b/src/core/masking.ts @@ -11,6 +11,8 @@ import { import type { FragmentMap } from "../utilities/index.js"; import type { ApolloCache, DocumentNode, TypedDocumentNode } from "./index.js"; import { invariant } from "../utilities/globals/index.js"; +import { equal } from "@wry/equality"; +import { Slot } from "optimism"; interface MaskingContext { operationType: "query" | "mutation" | "subscription" | "fragment"; @@ -20,6 +22,10 @@ interface MaskingContext { disableWarnings?: boolean; } +// Contextual slot that allows us to disable accessor warnings on fields when in +// migrate mode. +export const disableWarningsSlot = new Slot(); + export function maskOperation( data: TData, document: DocumentNode | TypedDocumentNode, @@ -59,9 +65,7 @@ export function maskOperation( ); if (Object.isFrozen(data)) { - context.disableWarnings = true; - maybeDeepFreeze(masked); - context.disableWarnings = false; + disableWarningsSlot.withValue(true, maybeDeepFreeze, [masked]); } return changed ? masked : data; @@ -110,6 +114,13 @@ export function maskFragment( return data; } + if (equal(data, {})) { + // Return early and skip the masking algorithm if we don't have any data + // yet. This can happen when cache.diff returns an empty object which is + // used from watchFragment. + return data; + } + const context: MaskingContext = { operationType: "fragment", operationName: fragment.name.value, @@ -124,9 +135,7 @@ export function maskFragment( ); if (Object.isFrozen(data)) { - context.disableWarnings = true; - maybeDeepFreeze(masked); - context.disableWarnings = false; + disableWarningsSlot.withValue(true, maybeDeepFreeze, [masked]); } return changed ? masked : data; @@ -370,8 +379,17 @@ function addAccessorWarning( path: string, context: MaskingContext ) { + // In order to preserve the original shape of the data as much as possible, we + // want to skip adding a property with warning to the final result when the + // value is missing, otherwise our final result will contain additional + // properties that our original result did not have. This could happen with a + // deferred payload for example. + if (value === void 0) { + return; + } + let getValue = () => { - if (context.disableWarnings) { + if (disableWarningsSlot.getValue()) { return value; }