From acb647e4efbddecf732b6e55dc47ac40c9bdaf08 Mon Sep 17 00:00:00 2001 From: Dylan Staley <88163+dstaley@users.noreply.github.com> Date: Wed, 29 Mar 2023 07:25:54 -0700 Subject: [PATCH] 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 Co-authored-by: Aleksandra --- .changeset/sharp-clouds-peel.md | 5 ++ .../src/client-side-base-visitor.ts | 47 +----------- .../tests/client-side-base-visitor.spec.ts | 75 ++++++++++++++++++- 3 files changed, 80 insertions(+), 47 deletions(-) create mode 100644 .changeset/sharp-clouds-peel.md diff --git a/.changeset/sharp-clouds-peel.md b/.changeset/sharp-clouds-peel.md new file mode 100644 index 00000000000..cb60de01d86 --- /dev/null +++ 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 index df6196f24e7..69b289a01c0 100644 --- 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>(); - - const deduplicatedImports = fragmentImports - .map(fragmentImport => { - const { path, identifiers } = fragmentImport.importSource; - if (!alreadyImported.has(path)) { - alreadyImported.set(path, new Set()); - } - - 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 index 2252d08747e..aa9916225e5 100644 --- 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'); + }); + }); + }); });