AP-682 Strip FragmentDefinitions in client:check when only included in @client fields

CHANGELOG.md

@@ -19,7 +19,7 @@
 - `apollo-graphql`
   - <First `apollo-graphql` related entry goes here>
 - `apollo-language-server`
-  - <First `apollo-language-server` related entry goes here>
+  - Fix issue where fragment definitions only included in `@client` fields would not be stripped ((AP-682)(https://golinks.io/AP-682), [#1454](https://github.com/apollographql/apollo-tooling/pull/1454))
 - `apollo-tools`
   - <First `apollo-tools` related entry goes here>
 - `vscode-apollo`

packages/apollo-language-server/src/utilities/__tests__/graphql.test.ts

@@ -1,6 +1,9 @@
 import gql from "graphql-tag";
-import { print } from "graphql";
-import { withTypenameFieldAddedWhereNeeded } from "../graphql";
+import { parse, print } from "graphql";
+import {
+  withTypenameFieldAddedWhereNeeded,
+  removeDirectiveAnnotatedFields
+} from "../graphql";
 
 describe("withTypenameFieldAddedWhereNeeded", () => {
   it("properly adds __typename to each selectionSet", () => {
@@ -73,3 +76,117 @@ describe("withTypenameFieldAddedWhereNeeded", () => {
     `);
   });
 });
+
+describe("removeDirectiveAnnotatedFields", () => {
+  it("should be a function", () => {
+    expect(typeof removeDirectiveAnnotatedFields).toBe("function");
+  });
+
+  it("should remove fields with matching directives", () => {
+    expect(
+      print(
+        removeDirectiveAnnotatedFields(
+          parse(`query Query { fieldToKeep fieldToRemove @client }`),
+          ["client"]
+        )
+      )
+    ).toMatchInlineSnapshot(`
+            "query Query {
+              fieldToKeep
+            }
+            "
+        `);
+  });
+
+  it("should remove object fields with matching directives", () => {
+    expect(
+      print(
+        removeDirectiveAnnotatedFields(
+          parse(`
+            query Query {
+              fieldToKeep
+              fieldToRemove @client {
+                childField
+              }
+            }
+          `),
+          ["client"]
+        )
+      )
+    ).toMatchInlineSnapshot(`
+      "query Query {
+        fieldToKeep
+      }
+      "
+    `);
+  });
+
+  it("should remove fragments that become unused when antecendant directives are removed", () => {
+    expect(
+      print(
+        removeDirectiveAnnotatedFields(
+          parse(`
+            fragment ClientObjectFragment on ClientObject {
+              string
+              number
+            }
+
+            fragment LaunchTile on Launch {
+              __typename
+              id
+              isBooked
+              rocket {
+                id
+                name
+              }
+              mission {
+                name
+                missionPatch
+              }
+            }
+
+            query LaunchDetails($launchId: ID!) {
+              launch(id: $launchId) {
+                isInCart @client
+                clientObject @client {
+                  ...ClientObjectFragment
+                }
+                site
+                rocket {
+                  type
+                }
+                ...LaunchTile
+              }
+            }
+          `),
+          ["client"]
+        )
+      )
+    ).toMatchInlineSnapshot(`
+      "fragment LaunchTile on Launch {
+        __typename
+        id
+        isBooked
+        rocket {
+          id
+          name
+        }
+        mission {
+          name
+          missionPatch
+        }
+      }
+
+      query LaunchDetails($launchId: ID!) {
+        launch(id: $launchId) {
+          site
+          rocket {
+            type
+          }
+          ...LaunchTile
+        }
+      }
+      "
+    `);
+  });
+});

packages/apollo-language-server/src/utilities/graphql.ts

@@ -88,12 +88,22 @@ export function removeDirectives(ast: ASTNode, directiveNames: string[]) {
 }
 
 // remove fields where a given directive is found
-export function removeDirectiveAnnotatedFields(
-  ast: ASTNode,
+export function removeDirectiveAnnotatedFields<AST extends ASTNode>(
+  ast: AST,
   directiveNames: string[]
-) {
+): AST {
   if (!directiveNames.length) return ast;
-  return visit(ast, {
+
+  /**
+   * List of names of all the `FragmentDefinition`s that we are going to keep.
+   *
+   * We store the names that we want to _keep_ instead of fields we want to _delete_ because it's possible
+   * that we're removing a `Fragment` from a `@client` field that is used in a non-`@client` field (if it's a
+   * fragment on an interface, for example).
+   */
+  const fragmentNamesToKeep = new Set<string>();
+
+  ast = visit(ast, {
     Field(node: FieldNode): FieldNode | null {
       if (
         node.directives &&
@@ -103,6 +113,18 @@ export function removeDirectiveAnnotatedFields(
         )
       )
         return null;
+
+      // We're keeping this field. Save the names of all fragment spreads that are in this field's selection
+      // set. This is intentionally _not_ recursive because descendents of this field may a directive that we
+      // want to remove; so we only know that this field's selection set is safe to keep.
+      if (node.selectionSet) {
+        node.selectionSet.selections.forEach(selection => {
+          if (selection.kind === "FragmentSpread") {
+            fragmentNamesToKeep.add(selection.name.value);
+          }
+        });
+      }
+
       return node;
     },
     OperationDefinition: {
@@ -112,6 +134,13 @@ export function removeDirectiveAnnotatedFields(
       }
     }
   });
+
+  // Remove all `FragmentDefinitions`s that have names _not_ stored in `fragmentNamesToKeep`.
+  return visit(ast, {
+    FragmentDefinition(node) {
+      return fragmentNamesToKeep.has(node.name.value) ? node : null;
+    }
+  });
 }
 
 const typenameField = {