diff --git a/.changeset/itchy-penguins-worry.md b/.changeset/itchy-penguins-worry.md new file mode 100644 index 00000000000..4459d490662 --- /dev/null +++ b/.changeset/itchy-penguins-worry.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix issue where masked data would sometimes get returned when the field was part of a child fragment from a fragment unmasked by the parent query. diff --git a/.changeset/mean-bottles-travel.md b/.changeset/mean-bottles-travel.md new file mode 100644 index 00000000000..a2e1fd300d2 --- /dev/null +++ b/.changeset/mean-bottles-travel.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix issue where the warning emitted by `@unmask(mode: "migrate")` would trigger unnecessarily when the fragment was used alongside a masked fragment inside an inline fragment. diff --git a/.changeset/wicked-pans-appear.md b/.changeset/wicked-pans-appear.md new file mode 100644 index 00000000000..fc4a3b5eb6d --- /dev/null +++ b/.changeset/wicked-pans-appear.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix issue that threw errors when masking partial data with `@unmask(mode: "migrate")`. diff --git a/.size-limits.json b/.size-limits.json index 45bba8c25f1..faaed486eb9 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 41638, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34394 + "dist/apollo-client.min.cjs": 41756, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34505 } diff --git a/src/core/__tests__/masking.test.ts b/src/core/__tests__/masking.test.ts index 3a6eb47251e..548d7ea0a2d 100644 --- a/src/core/__tests__/masking.test.ts +++ b/src/core/__tests__/masking.test.ts @@ -211,6 +211,107 @@ describe("maskOperation", () => { }); }); + test("masks fragments from nested objects when query gets fields from same object", () => { + const query = gql` + query { + user { + ...FragmentA @unmask + ...FragmentB + ...FragmentC @unmask + } + } + + fragment FragmentA on User { + this { + is { + very { + deeply { + nested { + a + } + } + } + } + } + } + fragment FragmentB on User { + this { + is { + very { + deeply { + nested { + b + } + } + } + } + } + } + fragment FragmentC on User { + this { + is { + very { + deeply { + nested { + c + } + } + } + } + } + } + `; + + const data = maskOperation( + deepFreeze({ + user: { + __typename: "User", + this: { + is: [ + { + very: { + deeply: [ + { + nested: { + a: 1, + b: 2, + c: 3, + }, + }, + ], + }, + }, + ], + }, + }, + }), + query, + new InMemoryCache() + ); + + expect(data).toEqual({ + user: { + __typename: "User", + this: { + is: [ + { + very: { + deeply: [ + { + nested: { + a: 1, + c: 3, + }, + }, + ], + }, + }, + ], + }, + }, + }); + }); + test("handles nulls in child selection sets", () => { const query = gql` query { @@ -718,140 +819,132 @@ describe("maskOperation", () => { }); }); - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'can handle fragment spreads in inline fragments with mix masked and @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); + test('can handle fragment spreads in inline fragments with mix masked and @unmask(mode: "migrate")', () => { + using _ = spyOnConsole("warn"); - const query = gql` - query GetUser { - user { - id - ... @defer { - ...UserFields - ...ProfileFields @unmask(mode: "migrate") - } + const query = gql` + query GetUser { + user { + id + ... @defer { + ...UserFields + ...ProfileFields @unmask(mode: "migrate") } } + } - fragment UserFields on User { - name - } - - fragment ProfileFields on User { - username - } - `; - - const data = maskOperation( - deepFreeze({ - user: { - __typename: "User", - id: 1, - name: "Test User", - username: "testuser", - }, - }), - query, - new InMemoryCache() - ); - - data.user.__typename; - data.user.id; - - expect(console.warn).not.toHaveBeenCalled(); - - data.user.username; + fragment UserFields on User { + name + } - expect(console.warn).toHaveBeenCalledTimes(1); - expect(console.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.", - "query 'GetUser'", - "user.username" - ); + fragment ProfileFields on User { + username + } + `; - expect(data).toEqual({ + const data = maskOperation( + deepFreeze({ user: { __typename: "User", id: 1, + name: "Test User", username: "testuser", }, - }); - } - ); - - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'can handle overlapping fragment spreads in inline fragments with mix masked and @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); - const query = gql` - query { - user { - id - ... { - ...UserFields - ...ProfileFields @unmask(mode: "migrate") - } - } - } + }), + query, + new InMemoryCache() + ); - fragment UserFields on User { - username - name - } + data.user.__typename; + data.user.id; - fragment ProfileFields on User { - username - email - } - `; + expect(console.warn).not.toHaveBeenCalled(); - const data = maskOperation( - deepFreeze({ - user: { - __typename: "User", - id: 1, - name: "Test User", - username: "testuser", - email: "testuser@example.com", - }, - }), - query, - new InMemoryCache() - ); + data.user.username; - data.user.__typename; - data.user.id; + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.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.", + "query 'GetUser'", + "user.username" + ); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + user: { + __typename: "User", + id: 1, + username: "testuser", + }, + }); + }); - data.user.username; - data.user.email; + test('can handle overlapping fragment spreads in inline fragments with mix masked and @unmask(mode: "migrate")', () => { + using _ = spyOnConsole("warn"); + const query = gql` + query GetUser { + user { + id + ... { + ...UserFields + ...ProfileFields @unmask(mode: "migrate") + } + } + } - expect(console.warn).toHaveBeenCalledTimes(2); - expect(console.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.", - "query 'GetUser'", - "user.username" - ); - expect(console.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.", - "query 'GetUser'", - "user.email" - ); + fragment UserFields on User { + username + name + } - expect(data).toEqual({ + fragment ProfileFields on User { + username + email + } + `; + + const data = maskOperation( + deepFreeze({ user: { __typename: "User", id: 1, + name: "Test User", username: "testuser", email: "testuser@example.com", }, - }); - } - ); + }), + query, + new InMemoryCache() + ); + + data.user.__typename; + data.user.id; + + expect(console.warn).not.toHaveBeenCalled(); + + data.user.username; + data.user.email; + + expect(console.warn).toHaveBeenCalledTimes(2); + expect(console.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.", + "query 'GetUser'", + "user.username" + ); + expect(console.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.", + "query 'GetUser'", + "user.email" + ); + + expect(data).toEqual({ + user: { + __typename: "User", + id: 1, + username: "testuser", + email: "testuser@example.com", + }, + }); + }); test("handles field aliases", () => { const query = gql` @@ -1993,7 +2086,7 @@ describe("maskOperation", () => { }); test('handles overlapping types when subtype has accessor warnings with @unmask(mode: "migrate")', async () => { - using consoleSpy = spyOnConsole("warn"); + using _ = spyOnConsole("warn"); const query = gql` query PlaylistQuery { playlist { @@ -2078,21 +2171,47 @@ describe("maskOperation", () => { new InMemoryCache() ); - expect(consoleSpy.warn).not.toHaveBeenCalled(); - - consoleSpy.warn.mockClear(); + expect(console.warn).not.toHaveBeenCalled(); data.playlist.album; data.playlist.album.id; data.playlist.album.__typename; + data.playlist.album.tracks[0].id; + data.playlist.album.tracks[0].__typename; data.playlist.artist; data.playlist.artist.id; data.playlist.artist.__typename; + data.playlist.artist.topTracks[0].id; + data.playlist.artist.topTracks[0].__typename; + expect(console.warn).not.toHaveBeenCalled(); data.playlist.album.images; + data.playlist.album.tracks[0].name; data.playlist.artist.images; - expect(console.warn).toHaveBeenCalledTimes(2); + data.playlist.artist.topTracks[0].name; + + expect(console.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.", + "query 'PlaylistQuery'", + "playlist.album.images" + ); + expect(console.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.", + "query 'PlaylistQuery'", + "playlist.album.tracks[0].name" + ); + expect(console.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.", + "query 'PlaylistQuery'", + "playlist.artist.images" + ); + expect(console.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.", + "query 'PlaylistQuery'", + "playlist.artist.topTracks[0].name" + ); + expect(console.warn).toHaveBeenCalledTimes(4); expect(data).toEqual({ playlist: { @@ -2432,104 +2551,109 @@ describe("maskOperation", () => { } }); - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'unmasks partial data with warnings with @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); + test('unmasks partial data with warnings with @unmask(mode: "migrate")', () => { + using consoleSpy = spyOnConsole("warn"); - const query = gql` - query { - greeting { - message - ...GreetingFragment @unmask(mode: "migrate") - } + const query = gql` + query { + greeting { + message + ...GreetingFragment @unmask(mode: "migrate") } + } - fragment GreetingFragment on Greeting { - sentAt - recipient { - name - } + fragment GreetingFragment on Greeting { + sentAt + recipient { + name } - `; + } + `; - { - const data = maskOperation( - deepFreeze({ - greeting: { message: "Hello world", __typename: "Greeting" }, - }), - query, - new InMemoryCache() - ); - - expect(data).toEqual({ + { + const data = maskOperation( + deepFreeze({ greeting: { message: "Hello world", __typename: "Greeting" }, - }); - } + }), + query, + new InMemoryCache() + ); - { - const data = maskOperation( - deepFreeze({ - greeting: { - __typename: "Greeting", - message: "Hello world", - sentAt: "2024-01-01", - }, - }), - query, - new InMemoryCache() - ); + data.greeting.__typename; + data.greeting.message; - data.greeting.__typename; - data.greeting.message; + expect(console.warn).not.toHaveBeenCalled(); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + greeting: { message: "Hello world", __typename: "Greeting" }, + }); + } - data.greeting.sentAt; - expect(console.warn).toHaveBeenCalledTimes(1); + consoleSpy.warn.mockClear(); - expect(data).toEqual({ + { + const data = maskOperation( + deepFreeze({ greeting: { __typename: "Greeting", message: "Hello world", sentAt: "2024-01-01", }, - }); - } + }), + query, + new InMemoryCache() + ); - { - const data = maskOperation( - deepFreeze({ - greeting: { - __typename: "Greeting", - message: "Hello world", - recipient: { __typename: "__Person" }, - }, - }), - query, - new InMemoryCache() - ); + data.greeting.__typename; + data.greeting.message; + + expect(console.warn).not.toHaveBeenCalled(); - data.greeting.__typename; - data.greeting.message; + data.greeting.sentAt; + expect(console.warn).toHaveBeenCalledTimes(1); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + greeting: { + __typename: "Greeting", + message: "Hello world", + sentAt: "2024-01-01", + }, + }); + } - data.greeting.recipient; - data.greeting.recipient.__typename; - expect(console.warn).toHaveBeenCalledTimes(1); + consoleSpy.warn.mockClear(); - expect(data).toEqual({ + { + const data = maskOperation( + deepFreeze({ greeting: { __typename: "Greeting", message: "Hello world", recipient: { __typename: "__Person" }, }, - }); - } + }), + query, + new InMemoryCache() + ); + + data.greeting.__typename; + data.greeting.message; + + expect(console.warn).not.toHaveBeenCalled(); + + data.greeting.recipient; + data.greeting.recipient.__typename; + expect(console.warn).toHaveBeenCalledTimes(1); + + expect(data).toEqual({ + greeting: { + __typename: "Greeting", + message: "Hello world", + recipient: { __typename: "__Person" }, + }, + }); } - ); + }); test("masks partial deferred data", () => { const query = gql` @@ -2646,80 +2770,82 @@ describe("maskOperation", () => { } }); - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'unmasks partial deferred data with warnings with @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); + test('unmasks partial deferred data with warnings with @unmask(mode: "migrate")', () => { + using consoleSpy = spyOnConsole("warn"); - const query = gql` - query { - greeting { - message - ... @defer { - sentAt - ...GreetingFragment @unmask(mode: "migrate") - } + const query = gql` + query { + greeting { + message + ... @defer { + sentAt + ...GreetingFragment @unmask(mode: "migrate") } } + } - fragment GreetingFragment on Greeting { - recipient { - name - } + fragment GreetingFragment on Greeting { + recipient { + name } - `; + } + `; - { - const data = maskOperation( - deepFreeze({ - greeting: { message: "Hello world", __typename: "Greeting" }, - }), - query, - new InMemoryCache() - ); - - expect(data).toEqual({ + { + const data = maskOperation( + deepFreeze({ greeting: { message: "Hello world", __typename: "Greeting" }, - }); - } + }), + query, + new InMemoryCache() + ); - { - const data = maskOperation( - deepFreeze({ - greeting: { - __typename: "Greeting", - message: "Hello world", - sentAt: "2024-01-01", - recipient: { __typename: "__Person", name: "Alice" }, - }, - }), - query, - new InMemoryCache() - ); + data.greeting.message; + data.greeting.__typename; - data.greeting.message; - data.greeting.sentAt; - data.greeting.__typename; + expect(console.warn).not.toHaveBeenCalled(); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + greeting: { message: "Hello world", __typename: "Greeting" }, + }); + } - data.greeting.recipient; - data.greeting.recipient.__typename; - data.greeting.recipient.name; - expect(console.warn).toHaveBeenCalledTimes(3); + consoleSpy.warn.mockClear(); - expect(data).toEqual({ + { + const data = maskOperation( + deepFreeze({ greeting: { __typename: "Greeting", message: "Hello world", sentAt: "2024-01-01", recipient: { __typename: "__Person", name: "Alice" }, }, - }); - } + }), + query, + new InMemoryCache() + ); + + data.greeting.message; + data.greeting.sentAt; + data.greeting.__typename; + + expect(console.warn).not.toHaveBeenCalled(); + + data.greeting.recipient; + data.greeting.recipient.name; + expect(console.warn).toHaveBeenCalledTimes(2); + + expect(data).toEqual({ + greeting: { + __typename: "Greeting", + message: "Hello world", + sentAt: "2024-01-01", + recipient: { __typename: "__Person", name: "Alice" }, + }, + }); } - ); + }); test("masks results with primitive arrays", () => { const query = gql` @@ -3496,8 +3622,7 @@ describe("maskFragment", () => { ); }); - // TODO: Remove .failing when refactoring migrate mode - test.failing("masks child fragments of @unmask", () => { + test("masks child fragments of @unmask", () => { using _ = spyOnConsole("warn"); const fragment = gql` @@ -3676,96 +3801,102 @@ describe("maskFragment", () => { } }); - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'unmasks partial data with warnings with @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); + test('unmasks partial data with warnings with @unmask(mode: "migrate")', () => { + using consoleSpy = spyOnConsole("warn"); - const fragment = gql` - fragment GreetingFields on Greeting { - message - ...AdditionalFields @unmask(mode: "migrate") - } + const fragment = gql` + fragment GreetingFields on Greeting { + message + ...AdditionalFields @unmask(mode: "migrate") + } - fragment AdditionalFields on Greeting { - sentAt - recipient { - name - } + fragment AdditionalFields on Greeting { + sentAt + recipient { + name } - `; - - { - const data = maskFragment( - deepFreeze({ message: "Hello world", __typename: "Greeting" }), - fragment, - new InMemoryCache(), - "GreetingFields" - ); - - expect(data).toEqual({ - message: "Hello world", - __typename: "Greeting", - }); } + `; - { - const data = maskFragment( - deepFreeze({ - __typename: "Greeting", - message: "Hello world", - sentAt: "2024-01-01", - }), - fragment, - new InMemoryCache(), - "GreetingFields" - ); + { + const data = maskFragment( + deepFreeze({ message: "Hello world", __typename: "Greeting" }), + fragment, + new InMemoryCache(), + "GreetingFields" + ); - data.__typename; - data.message; + data.message; + data.__typename; + + expect(console.warn).not.toHaveBeenCalled(); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + message: "Hello world", + __typename: "Greeting", + }); + } - data.sentAt; - expect(console.warn).toHaveBeenCalledTimes(1); + consoleSpy.warn.mockClear(); - expect(data).toEqual({ + { + const data = maskFragment( + deepFreeze({ __typename: "Greeting", message: "Hello world", sentAt: "2024-01-01", - }); - } + }), + fragment, + new InMemoryCache(), + "GreetingFields" + ); - { - const data = maskFragment( - deepFreeze({ - __typename: "Greeting", - message: "Hello world", - recipient: { __typename: "__Person" }, - }), - fragment, - new InMemoryCache(), - "GreetingFields" - ); + data.__typename; + data.message; - data.__typename; - data.message; + expect(console.warn).not.toHaveBeenCalled(); - expect(console.warn).not.toHaveBeenCalled(); + data.sentAt; + expect(console.warn).toHaveBeenCalledTimes(1); - data.recipient; - data.recipient.__typename; - expect(console.warn).toHaveBeenCalledTimes(1); + expect(data).toEqual({ + __typename: "Greeting", + message: "Hello world", + sentAt: "2024-01-01", + }); + } - expect(data).toEqual({ + consoleSpy.warn.mockClear(); + + { + const data = maskFragment( + deepFreeze({ __typename: "Greeting", message: "Hello world", recipient: { __typename: "__Person" }, - }); - } + }), + fragment, + new InMemoryCache(), + "GreetingFields" + ); + + data.__typename; + data.message; + + expect(console.warn).not.toHaveBeenCalled(); + + data.recipient; + // We do not warn on access to __typename + data.recipient.__typename; + expect(console.warn).toHaveBeenCalledTimes(1); + + expect(data).toEqual({ + __typename: "Greeting", + message: "Hello world", + recipient: { __typename: "__Person" }, + }); } - ); + }); test("masks partial deferred data", () => { const fragment = gql` @@ -3872,75 +4003,74 @@ describe("maskFragment", () => { } }); - // TODO: Remove .failing when refactoring migrate mode - test.failing( - 'unmasks partial deferred data with warnings with @unmask(mode: "migrate")', - () => { - using _ = spyOnConsole("warn"); + test('unmasks partial deferred data with warnings with @unmask(mode: "migrate")', () => { + using consoleSpy = spyOnConsole("warn"); - const fragment = gql` - fragment GreetingFields on Greeting { - message - ... @defer { - sentAt - ...AdditionalFields @unmask(mode: "migrate") - } + const fragment = gql` + fragment GreetingFields on Greeting { + message + ... @defer { + sentAt + ...AdditionalFields @unmask(mode: "migrate") } + } - fragment AdditionalFields on Greeting { - recipient { - name - } + fragment AdditionalFields on Greeting { + recipient { + name } - `; - - { - const data = maskFragment( - deepFreeze({ message: "Hello world", __typename: "Greeting" }), - fragment, - new InMemoryCache(), - "GreetingFields" - ); - - expect(data).toEqual({ - message: "Hello world", - __typename: "Greeting", - }); } + `; - { - const data = maskFragment( - deepFreeze({ - __typename: "Greeting", - message: "Hello world", - sentAt: "2024-01-01", - recipient: { __typename: "__Person", name: "Alice" }, - }), - fragment, - new InMemoryCache(), - "GreetingFields" - ); - - data.message; - data.sentAt; - data.__typename; + { + const data = maskFragment( + deepFreeze({ message: "Hello world", __typename: "Greeting" }), + fragment, + new InMemoryCache(), + "GreetingFields" + ); - expect(console.warn).not.toHaveBeenCalled(); + expect(data).toEqual({ + message: "Hello world", + __typename: "Greeting", + }); + } - data.recipient; - data.recipient.__typename; - data.recipient.name; - expect(console.warn).toHaveBeenCalledTimes(3); + consoleSpy.warn.mockClear(); - expect(data).toEqual({ + { + const data = maskFragment( + deepFreeze({ __typename: "Greeting", message: "Hello world", sentAt: "2024-01-01", recipient: { __typename: "__Person", name: "Alice" }, - }); - } + }), + fragment, + new InMemoryCache(), + "GreetingFields" + ); + + data.message; + data.sentAt; + data.__typename; + + expect(console.warn).not.toHaveBeenCalled(); + + data.recipient; + data.recipient.name; + // We do not warn on access to __typename + data.recipient.__typename; + expect(console.warn).toHaveBeenCalledTimes(2); + + expect(data).toEqual({ + __typename: "Greeting", + message: "Hello world", + sentAt: "2024-01-01", + recipient: { __typename: "__Person", name: "Alice" }, + }); } - ); + }); test("masks results with primitive arrays", () => { const fragment = gql` diff --git a/src/core/masking.ts b/src/core/masking.ts index fdede0508ac..0fa811ad755 100644 --- a/src/core/masking.ts +++ b/src/core/masking.ts @@ -1,9 +1,5 @@ import { Kind } from "graphql"; -import type { - FragmentDefinitionNode, - SelectionNode, - SelectionSetNode, -} from "graphql"; +import type { FragmentDefinitionNode, SelectionSetNode } from "graphql"; import { createFragmentMap, resultKeyNameFromField, @@ -11,6 +7,8 @@ import { getFragmentMaskMode, getOperationDefinition, maybeDeepFreeze, + canUseWeakMap, + canUseWeakSet, } from "../utilities/index.js"; import type { FragmentMap } from "../utilities/index.js"; import type { ApolloCache, DocumentNode, TypedDocumentNode } from "./index.js"; @@ -23,8 +21,13 @@ interface MaskingContext { operationName: string | undefined; fragmentMap: FragmentMap; cache: ApolloCache; + mutableTargets: WeakMap; + knownChanged: WeakSet; } +const MapImpl = canUseWeakMap ? WeakMap : Map; +const SetImpl = canUseWeakSet ? WeakSet : Set; + // Contextual slot that allows us to disable accessor warnings on fields when in // migrate mode. export const disableWarningsSlot = new Slot(); @@ -59,17 +62,23 @@ export function maskOperation( operationName: definition.name?.value, fragmentMap: createFragmentMap(getFragmentDefinitions(document)), cache, + mutableTargets: new MapImpl(), + knownChanged: new SetImpl(), }; - const [masked, changed] = maskSelectionSet( - data, - definition.selectionSet, - context - ); + const [masked, changed] = disableWarningsSlot.withValue(true, () => { + const maskedTuple = maskSelectionSet( + data, + definition.selectionSet, + context, + false + ); - if (Object.isFrozen(data)) { - disableWarningsSlot.withValue(true, maybeDeepFreeze, [masked]); - } + if (Object.isFrozen(data)) { + maybeDeepFreeze(maskedTuple[0]); + } + return maskedTuple; + }); return changed ? masked : data; } @@ -129,290 +138,209 @@ export function maskFragment( operationName: fragment.name.value, fragmentMap: createFragmentMap(getFragmentDefinitions(document)), cache, + mutableTargets: new MapImpl(), + knownChanged: new SetImpl(), }; - const [masked, changed] = maskSelectionSet( - data, - fragment.selectionSet, - context - ); + const [masked, changed] = disableWarningsSlot.withValue(true, () => { + const maskedTuple = maskSelectionSet( + data, + fragment.selectionSet, + context, + false + ); - if (Object.isFrozen(data)) { - disableWarningsSlot.withValue(true, maybeDeepFreeze, [masked]); - } + if (Object.isFrozen(data)) { + maybeDeepFreeze(maskedTuple[0]); + } + return maskedTuple; + }); return changed ? masked : data; } +function getMutableTarget( + data: Record, + context: MaskingContext +): typeof data { + if (context.mutableTargets.has(data)) { + return context.mutableTargets.get(data); + } + + const mutableTarget = Array.isArray(data) ? [] : Object.create(null); + context.mutableTargets.set(data, mutableTarget); + return mutableTarget; +} + function maskSelectionSet( data: any, selectionSet: SelectionSetNode, context: MaskingContext, + migration: boolean, path?: string | undefined ): [data: any, changed: boolean] { if (Array.isArray(data)) { let changed = false; - - const masked = data.map((item, index) => { + const target = getMutableTarget(data, context); + for (const [index, item] of Array.from(data.entries())) { if (item === null) { - return null; + target[index] = null; + continue; } const [masked, itemChanged] = maskSelectionSet( item, selectionSet, context, + migration, __DEV__ ? `${path || ""}[${index}]` : void 0 ); changed ||= itemChanged; - return itemChanged ? masked : item; - }); + target[index] = masked; + } - return [changed ? masked : data, changed]; + return [changed ? target : data, changed]; } - const result = selectionSet.selections - .concat() - .sort(sortFragmentsLast) - .reduce<[any, boolean]>( - ([memo, changed], selection) => { - switch (selection.kind) { - case Kind.FIELD: { - const keyName = resultKeyNameFromField(selection); - const childSelectionSet = selection.selectionSet; - - memo[keyName] = data[keyName]; - - if (memo[keyName] === void 0) { - delete memo[keyName]; - } - - if ( - keyName in memo && - childSelectionSet && - data[keyName] !== null - ) { - const [masked, childChanged] = maskSelectionSet( - data[keyName], - childSelectionSet, - context, - __DEV__ ? `${path || ""}.${keyName}` : void 0 - ); - - if ( - childChanged || - // This check prevents cases where masked fields may accidentally be - // returned as part of this object when the fragment also selects - // additional fields from the same child selection. - Object.keys(masked).length !== Object.keys(data[keyName]).length - ) { - memo[keyName] = masked; - changed = true; - } - } - - return [memo, changed]; - } - case Kind.INLINE_FRAGMENT: { - if ( - selection.typeCondition && - !context.cache.fragmentMatches!(selection, data.__typename) - ) { - return [memo, changed]; - } - - const [fragmentData, childChanged] = maskSelectionSet( - data, - selection.selectionSet, - context, - path - ); - - return [ - { - ...memo, - ...fragmentData, - }, - changed || childChanged, - ]; - } - case Kind.FRAGMENT_SPREAD: { - const fragmentName = selection.name.value; - let fragment: FragmentDefinitionNode | null = - context.fragmentMap[fragmentName] || - (context.fragmentMap[fragmentName] = - context.cache.lookupFragment(fragmentName)!); - invariant( - fragment, - "Could not find fragment with name '%s'.", - fragmentName - ); - - const mode = getFragmentMaskMode(selection); - - if (mode === "mask") { - return [memo, true]; - } - - if (__DEV__) { - if (mode === "migrate") { - return [ - addFieldAccessorWarnings( - memo, - data, - fragment.selectionSet, - path || "", - context - ), - true, - ]; - } - } - - const [fragmentData, changed] = maskSelectionSet( - data, - fragment.selectionSet, - context, - path - ); + let changed = false; + const memo = getMutableTarget(data, context); + for (const selection of selectionSet.selections) { + switch (selection.kind) { + case Kind.FIELD: { + const keyName = resultKeyNameFromField(selection); + const childSelectionSet = selection.selectionSet; + + let newValue = memo[keyName] || data[keyName]; + if (keyName in data && childSelectionSet && data[keyName] !== null) { + const [masked, childChanged] = maskSelectionSet( + data[keyName], + childSelectionSet, + context, + migration, + __DEV__ ? `${path || ""}.${keyName}` : void 0 + ); - return [{ ...memo, ...fragmentData }, changed]; + if (childChanged) { + newValue = masked; + changed = true; } } - }, - [Object.create(null), false] - ); - if (data && "__typename" in data && !("__typename" in result[0])) { - result[0].__typename = data.__typename; - } - - return result; -} - -function addFieldAccessorWarnings( - memo: Record, - data: Record, - selectionSetNode: SelectionSetNode, - path: string, - context: MaskingContext -) { - if (Array.isArray(data)) { - return data.map((item, index): unknown => { - return addFieldAccessorWarnings( - memo[index] || Object.create(null), - item, - selectionSetNode, - `${path}[${index}]`, - context - ); - }); - } - - return selectionSetNode.selections - .concat() - .sort(sortFragmentsLast) - .reduce((memo, selection) => { - switch (selection.kind) { - case Kind.FIELD: { - const keyName = resultKeyNameFromField(selection); - const childSelectionSet = selection.selectionSet; - - if (keyName in memo && !childSelectionSet) { - return memo; - } - - let value = data[keyName]; - - if (childSelectionSet) { - value = addFieldAccessorWarnings( - memo[keyName] || - (Array.isArray(data[keyName]) ? [] : Object.create(null)), - data[keyName] as Record, - childSelectionSet, - `${path}.${keyName}`, - context - ); + if (newValue !== void 0) { + if (!__DEV__) { + memo[keyName] = newValue; } - if (__DEV__) { - if (keyName in memo) { - memo[keyName] = value; + if ( + migration && + keyName !== "__typename" && + // either the field is not present in the memo object + // or it has a `get` descriptor, not a `value` descriptor + // => it is a warning accessor and we can overwrite it + // with another accessor + !Object.getOwnPropertyDescriptor(memo, keyName)?.value + ) { + Object.defineProperty( + memo, + keyName, + getAccessorWarningDescriptor( + keyName, + newValue, + path || "", + context.operationName, + context.operationType + ) + ); } else { - addAccessorWarning(memo, value, keyName, path, context); + delete memo[keyName]; + memo[keyName] = newValue; } } - - if (!__DEV__) { - memo[keyName] = data[keyName]; - } - - return memo; } - case Kind.INLINE_FRAGMENT: { - if ( - selection.typeCondition && - !context.cache.fragmentMatches!(selection, (data as any).__typename) - ) { - return memo; - } + // we later want to add acessor warnings to the final result + // so we need a new object to add the accessor warning to + changed ||= migration; + break; + } + case Kind.INLINE_FRAGMENT: { + if ( + selection.typeCondition && + !context.cache.fragmentMatches!(selection, data.__typename) + ) { + break; + } - return addFieldAccessorWarnings( - memo, - data, - selection.selectionSet, - path, - context - ); + const [, childChanged] = maskSelectionSet( + data, + selection.selectionSet, + context, + migration, + path + ); + changed ||= childChanged; + break; + } + case Kind.FRAGMENT_SPREAD: { + const fragmentName = selection.name.value; + let fragment: FragmentDefinitionNode | null = + context.fragmentMap[fragmentName] || + (context.fragmentMap[fragmentName] = + context.cache.lookupFragment(fragmentName)!); + invariant( + fragment, + "Could not find fragment with name '%s'.", + fragmentName + ); + + const mode = getFragmentMaskMode(selection); + + if (mode === "mask") { + break; } - case Kind.FRAGMENT_SPREAD: { - const fragment = context.fragmentMap[selection.name.value]; - const mode = getFragmentMaskMode(selection); - if (mode === "mask") { - return memo; - } + const [, selectionChanged] = maskSelectionSet( + data, + fragment.selectionSet, + context, + mode === "migrate", + path + ); - if (mode === "unmask") { - const [fragmentData] = maskSelectionSet( - data, - fragment.selectionSet, - context, - path - ); + changed ||= selectionChanged; + break; + } + } + } - return Object.assign(memo, fragmentData); - } + if (data && "__typename" in data && !("__typename" in memo)) { + memo.__typename = data.__typename; + } - return addFieldAccessorWarnings( - memo, - data, - fragment.selectionSet, - path, - context - ); - } - } - }, memo); + // This check prevents cases where masked fields may accidentally be + // returned as part of this object when the fragment also selects + // additional fields from the same child selection. + changed ||= Object.keys(memo).length !== Object.keys(data).length; + + // If the object has been changed in another subtree, but not in this, + // we still have to return "changed" as true, as otherwise a call parent + // would use original data instead of the masked one. + if (changed) { + context.knownChanged.add(memo); + } else { + changed ||= context.knownChanged.has(memo); + } + + return [changed ? memo : data, changed]; } -function addAccessorWarning( - data: Record, - value: any, +function getAccessorWarningDescriptor( fieldName: string, + value: any, 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; - } - + operationName: string | undefined, + operationType: string +): PropertyDescriptor { let getValue = () => { if (disableWarningsSlot.getValue()) { return value; @@ -420,9 +348,9 @@ function addAccessorWarning( invariant.warn( "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.", - context.operationName ? - `${context.operationType} '${context.operationName}'` - : `anonymous ${context.operationType}`, + operationName ? + `${operationType} '${operationName}'` + : `anonymous ${operationType}`, `${path}.${fieldName}`.replace(/^\./, "") ); @@ -431,16 +359,17 @@ function addAccessorWarning( return value; }; - Object.defineProperty(data, fieldName, { + return { get() { return getValue(); }, - set(value) { + set(v) { + value = v; getValue = () => value; }, enumerable: true, configurable: true, - }); + }; } let issuedWarning = false; @@ -452,11 +381,3 @@ function warnOnImproperCacheImplementation() { ); } } - -function sortFragmentsLast(a: SelectionNode, b: SelectionNode) { - if (a.kind === b.kind) { - return 0; - } - - return a.kind === Kind.FRAGMENT_SPREAD ? 1 : -1; -}