Get data output correctly. Need to fix warnings

apollographql · Nov 15, 2024 · 8848206 · 8848206
1 parent 49308ce
commit 8848206
Show file tree

Hide file tree

Showing 2 changed files with 120 additions and 35 deletions.
diff --git a/src/core/__tests__/masking.test.ts b/src/core/__tests__/masking.test.ts
@@ -1718,7 +1718,7 @@ describe("maskOperation", () => {
   });
 
   test('handles overlapping types when subtype has accessor warnings with @unmask(mode: "migrate")', async () => {
-    using _ = spyOnConsole("warn");
+    using consoleSpy = spyOnConsole("warn");
     const query = gql`
       query PlaylistQuery {
         playlist {
@@ -1727,10 +1727,18 @@ describe("maskOperation", () => {
           name
           album {
             id
+            tracks {
+              id
+              __typename
+            }
             __typename
           }
           artist {
             id
+            topTracks {
+              id
+              __typename
+            }
             __typename
           }
           __typename
@@ -1746,6 +1754,11 @@ describe("maskOperation", () => {
             url
             __typename
           }
+          tracks {
+            id
+            name
+            __typename
+          }
           __typename
         }
       }
@@ -1757,6 +1770,11 @@ describe("maskOperation", () => {
             url
             __typename
           }
+          topTracks {
+            id
+            name
+            __typename
+          }
           __typename
         }
       }
@@ -1771,20 +1789,36 @@ describe("maskOperation", () => {
             id: "2RSIoPew2TOy41ASHpzOx3",
             __typename: "Album",
             images: [{ url: "https://i.scdn.co/image/1", __typename: "Image" }],
+            tracks: [{ id: "1", name: "Track 1", __typename: "Track" }],
           },
           artist: {
             id: "2",
             __typename: "Artist",
             images: [{ url: "https://i.scdn.co/image/1", __typename: "Image" }],
+            topTracks: [{ id: "2", name: "Track 2", __typename: "Track" }],
           },
         },
       },
       query,
       new InMemoryCache()
     );
 
+    expect(consoleSpy.warn).not.toHaveBeenCalled();
+
+    consoleSpy.warn.mockClear();
+
+    data.playlist.album;
+    data.playlist.album.id;
+    data.playlist.album.__typename;
+    data.playlist.artist;
+    data.playlist.artist.id;
+    data.playlist.artist.__typename;
     expect(console.warn).not.toHaveBeenCalled();
 
+    data.playlist.album.images;
+    data.playlist.artist.images;
+    expect(console.warn).toHaveBeenCalledTimes(2);
+
     expect(data).toEqual({
       playlist: {
         id: "1",
@@ -1793,26 +1827,16 @@ describe("maskOperation", () => {
           id: "2RSIoPew2TOy41ASHpzOx3",
           __typename: "Album",
           images: [{ url: "https://i.scdn.co/image/1", __typename: "Image" }],
+          tracks: [{ id: "1", name: "Track 1", __typename: "Track" }],
         },
         artist: {
           id: "2",
           __typename: "Artist",
           images: [{ url: "https://i.scdn.co/image/1", __typename: "Image" }],
+          topTracks: [{ id: "2", name: "Track 2", __typename: "Track" }],
         },
       },
     });
-
-    data.playlist.album;
-    data.playlist.album.id;
-    data.playlist.album.__typename;
-    data.playlist.artist;
-    data.playlist.artist.id;
-    data.playlist.artist.__typename;
-    expect(console.warn).not.toHaveBeenCalled();
-
-    data.playlist.album.images;
-    data.playlist.artist.images;
-    expect(console.warn).toHaveBeenCalledTimes(2);
   });
 
   test("masks fragments in subscription documents", () => {

diff --git a/src/core/masking.ts b/src/core/masking.ts
@@ -7,6 +7,7 @@ import {
   getFragmentMaskMode,
   getOperationDefinition,
   maybeDeepFreeze,
+  isNonNullObject,
 } from "../utilities/index.js";
 import type { FragmentMap } from "../utilities/index.js";
 import type { ApolloCache, DocumentNode, TypedDocumentNode } from "./index.js";
@@ -173,34 +174,45 @@ function maskSelectionSet(
     ([memo, changed], selection) => {
       switch (selection.kind) {
         case Kind.FIELD: {
-          const keyName = resultKeyNameFromField(selection);
-          const childSelectionSet = selection.selectionSet;
+          disableWarningsSlot.withValue(true, () => {
+            const keyName = resultKeyNameFromField(selection);
+            const childSelectionSet = selection.selectionSet;
 
-          memo[keyName] = data[keyName];
-
-          if (memo[keyName] === void 0) {
-            delete memo[keyName];
-          }
+            const maybeValueWithWarning = memo[keyName];
+            memo[keyName] = data[keyName];
 
-          if (keyName in memo && childSelectionSet && data[keyName] !== null) {
-            const [masked, childChanged] = maskSelectionSet(
-              data[keyName],
-              childSelectionSet,
-              context,
-              __DEV__ ? `${path || ""}.${keyName}` : void 0
-            );
+            if (memo[keyName] === void 0) {
+              delete memo[keyName];
+            }
 
             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
+              keyName in memo &&
+              childSelectionSet &&
+              data[keyName] !== null
             ) {
-              memo[keyName] = masked;
-              changed = true;
+              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
+              ) {
+                delete memo[keyName];
+                memo[keyName] =
+                  maybeValueWithWarning ?
+                    backfillFieldsWithWarnings(masked, maybeValueWithWarning)
+                  : masked;
+                changed = true;
+              }
             }
-          }
+          });
 
           return [memo, changed];
         }
@@ -435,3 +447,52 @@ function warnOnImproperCacheImplementation() {
     );
   }
 }
+
+function backfillFieldsWithWarnings(
+  source: Record<string, unknown>,
+  unmasked: Record<string, unknown>
+): any {
+  if (Array.isArray(source)) {
+    if (Array.isArray(unmasked)) {
+      return source.map((s, idx) =>
+        backfillFieldsWithWarnings(s, unmasked[idx])
+      );
+    }
+
+    return source;
+  }
+
+  const keys = new Set([...Object.keys(source), ...Object.keys(unmasked)]);
+  const returnValue: Record<string, unknown> = {};
+  const { hasOwnProperty } = Object.prototype;
+
+  if (isNonNullObject(source) && isNonNullObject(unmasked)) {
+    Array.from(keys).forEach((key) => {
+      if (hasOwnProperty.call(source, key)) {
+        if (isNonNullObject(source[key]) && isNonNullObject(unmasked[key])) {
+          returnValue[key] = backfillFieldsWithWarnings(
+            source[key],
+            unmasked[key]
+          );
+        } else {
+          returnValue[key] = source[key];
+        }
+
+        return;
+      }
+
+      const descriptor = Object.getOwnPropertyDescriptor(unmasked, key);
+
+      Object.defineProperty(returnValue, key, {
+        get: descriptor?.get,
+        set: descriptor?.set,
+        enumerable: true,
+        configurable: true,
+      });
+    });
+  } else {
+    return source;
+  }
+
+  return returnValue;
+}