From fe0b18041499ff84e028d97a250e6c3d7cc92daa Mon Sep 17 00:00:00 2001 From: Adrien HARNAY Date: Wed, 8 Mar 2023 16:09:23 +0100 Subject: [PATCH 1/4] Stop importing fragments that are now unused --- .../src/client-side-base-visitor.ts | 44 ------------------- 1 file changed, 44 deletions(-) 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 bba119b7e00..0fd3896eac3 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 @@ -604,50 +604,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); } From a746a42311e377b1d11e8db0f97226abeb57afa7 Mon Sep 17 00:00:00 2001 From: Dylan Staley <88163+dstaley@users.noreply.github.com> Date: Thu, 16 Mar 2023 13:49:16 -0700 Subject: [PATCH 2/4] fix: remove unused argument and import --- .../visitor-plugin-common/src/client-side-base-visitor.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 0fd3896eac3..da322869076 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'; @@ -553,7 +552,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) { From 6580bbb3d1dcc28f806c82dd30f73aab42e35481 Mon Sep 17 00:00:00 2001 From: Dylan Staley <88163+dstaley@users.noreply.github.com> Date: Thu, 16 Mar 2023 14:05:18 -0700 Subject: [PATCH 3/4] chore: add changeset --- .changeset/sharp-clouds-peel.md | 5 +++++ 1 file changed, 5 insertions(+) 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 From e444eb8da836c95d688409be7644d54cf6397947 Mon Sep 17 00:00:00 2001 From: Dylan Staley <88163+dstaley@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:56:49 -0700 Subject: [PATCH 4/4] feat: add tests for inlined fragments --- .../tests/client-side-base-visitor.spec.ts | 75 ++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) 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'); + }); + }); + }); });