Stop importing fragments that are now unused (#9194)

* Stop importing fragments that are now unused * fix: remove unused argument and import * chore: add changeset * feat: add tests for inlined fragments --------- Co-authored-by: Adrien HARNAY <[email protected]> Co-authored-by: Aleksandra <[email protected]>
dotansimha · Mar 29, 2023 · acb647e · acb647e
1 parent e347e36
commit acb647e
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 47 deletions.
diff --git a/.changeset/sharp-clouds-peel.md b/.changeset/sharp-clouds-peel.md
@@ -0,0 +1,5 @@
+---
+'@graphql-codegen/visitor-plugin-common': patch
+---
+
+Don't emit import statements for unused fragments
diff --git a/packages/plugins/other/visitor-plugin-common/src/client-side-base-visitor.ts b/packages/plugins/other/visitor-plugin-common/src/client-side-base-visitor.ts
@@ -15,7 +15,6 @@ import {
 } from 'graphql';
 import gqlTag from 'graphql-tag';
 import { BaseVisitor, ParsedConfig, RawConfig } from './base-visitor.js';
-import { generateFragmentImportStatement } from './imports.js';
 import { LoadedFragment, ParsedImport } from './types.js';
 import { buildScalarsFromConfig, getConfigValue } from './utils.js';
 
@@ -569,7 +568,7 @@ export class ClientSideBaseVisitor<
     return path;
   }
 
-  public getImports(options: { excludeFragments?: boolean } = {}): string[] {
+  public getImports(): string[] {
     (this._additionalImports || []).forEach(i => this._imports.add(i));
 
     switch (this.config.documentMode) {
@@ -620,50 +619,6 @@ export class ClientSideBaseVisitor<
         break;
     }
 
-    if (!options.excludeFragments && !this.config.globalNamespace) {
-      const { documentMode, fragmentImports } = this.config;
-      if (
-        documentMode === DocumentMode.graphQLTag ||
-        documentMode === DocumentMode.string ||
-        documentMode === DocumentMode.documentNodeImportFragments
-      ) {
-        // keep track of what imports we've already generated so we don't try
-        // to import the same identifier twice
-        const alreadyImported = new Map<string, Set<string>>();
-
-        const deduplicatedImports = fragmentImports
-          .map(fragmentImport => {
-            const { path, identifiers } = fragmentImport.importSource;
-            if (!alreadyImported.has(path)) {
-              alreadyImported.set(path, new Set<string>());
-            }
-
-            const alreadyImportedForPath = alreadyImported.get(path);
-            const newIdentifiers = identifiers.filter(identifier => !alreadyImportedForPath.has(identifier.name));
-            newIdentifiers.forEach(newIdentifier => alreadyImportedForPath.add(newIdentifier.name));
-
-            // filter the set of identifiers in this fragment import to only
-            // the ones we haven't already imported from this path
-            return {
-              ...fragmentImport,
-              importSource: {
-                ...fragmentImport.importSource,
-                identifiers: newIdentifiers,
-              },
-              emitLegacyCommonJSImports: this.config.emitLegacyCommonJSImports,
-            };
-          })
-          // remove any imports that now have no identifiers in them
-          .filter(fragmentImport => fragmentImport.importSource.identifiers.length > 0);
-
-        deduplicatedImports.forEach(fragmentImport => {
-          if (fragmentImport.outputPath !== fragmentImport.importSource.path) {
-            this._imports.add(generateFragmentImportStatement(fragmentImport, 'document'));
-          }
-        });
-      }
-    }
-
     return Array.from(this._imports);
   }
 

diff --git a/packages/plugins/other/visitor-plugin-common/tests/client-side-base-visitor.spec.ts b/packages/plugins/other/visitor-plugin-common/tests/client-side-base-visitor.spec.ts
@@ -1,4 +1,4 @@
-import { buildSchema, OperationDefinitionNode, parse } from 'graphql';
+import { buildSchema, FragmentDefinitionNode, OperationDefinitionNode, parse, Kind } from 'graphql';
 import { ClientSideBaseVisitor, DocumentMode } from '../src/client-side-base-visitor.js';
 
 describe('getImports', () => {
@@ -129,4 +129,77 @@ describe('getImports', () => {
       });
     });
   });
+
+  describe('when documentMode "documentNodeImportFragments"', () => {
+    const schema = buildSchema(/* GraphQL */ `
+      type Query {
+        a: A
+      }
+
+      type A {
+        foo: String
+        bar: String
+      }
+    `);
+
+    it('does not import FragmentDocs', () => {
+      const fileName = 'fooBarQuery';
+      const importPath = `src/queries/${fileName}`;
+
+      const document = parse(
+        `query fooBarQuery {
+          a {
+            ...fields
+          }
+        }
+        fragment fields on A {
+          foo
+          bar
+        }
+      `
+      );
+
+      const visitor = new ClientSideBaseVisitor(
+        schema,
+        (document.definitions.filter(d => d.kind === Kind.FRAGMENT_DEFINITION) as FragmentDefinitionNode[]).map(
+          fragmentDef => ({
+            node: fragmentDef,
+            name: fragmentDef.name.value,
+            onType: fragmentDef.typeCondition.name.value,
+            isExternal: false,
+          })
+        ),
+        {
+          emitLegacyCommonJSImports: true,
+          importDocumentNodeExternallyFrom: 'near-operation-file',
+          documentMode: DocumentMode.documentNodeImportFragments,
+          fragmentImports: [
+            {
+              baseDir: '/',
+              baseOutputDir: '',
+              outputPath: '',
+              importSource: {
+                path: '~types',
+                identifiers: [
+                  { name: 'FieldsFragmentDoc', kind: 'document' },
+                  { name: 'FieldsFragment', kind: 'type' },
+                ],
+              },
+              emitLegacyCommonJSImports: true,
+              typesImport: false,
+            },
+          ],
+        },
+        {},
+        [{ document, location: importPath }]
+      );
+
+      visitor.OperationDefinition(document.definitions[0] as OperationDefinitionNode);
+
+      const imports = visitor.getImports();
+      imports.forEach(i => {
+        expect(i).not.toContain('FragmentDoc');
+      });
+    });
+  });
 });