[known-fragment-names]: fix when fragment used on interface and union (…

…#991) * [known-fragment-names]: fix when fragment used on interface and union * fix build
dimaMachina · Mar 14, 2022 · 56d6ff3 · 56d6ff3
1 parent 3fe3761
commit 56d6ff3
Show file tree

Hide file tree

Showing 5 changed files with 118 additions and 48 deletions.
diff --git a/.changeset/neat-apes-doubt.md b/.changeset/neat-apes-doubt.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+[known-fragment-names]: fix when fragment used on interface and union
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -60,7 +60,7 @@ jobs:
             ${{runner.os}}-16-8-${{matrix.graphql_version}}-node-modules-${{hashFiles('yarn.lock')}}
             ${{runner.os}}-16-8-${{matrix.graphql_version}}-node-modules-
       - name: Use GraphQL v${{matrix.graphql_version}}
-        run: node ./scripts/match-graphql.js ${{matrix.graphql_version}}
+        run: node ./scripts/match-graphql.mjs ${{matrix.graphql_version}}
       - name: Install Dependencies using Yarn
         run: yarn install && git checkout yarn.lock
       - name: Build
@@ -102,7 +102,7 @@ jobs:
             ${{runner.os}}-${{matrix.node_version}}-${{matrix.eslint_version}}-${{matrix.graphql_version}}-node-modules-${{hashFiles('yarn.lock')}}
             ${{runner.os}}-${{matrix.node_version}}-${{matrix.eslint_version}}-${{matrix.graphql_version}}-node-modules-
       - name: Use GraphQL v${{matrix.graphql_version}}
-        run: node ./scripts/match-graphql.js ${{matrix.graphql_version}}
+        run: node ./scripts/match-graphql.mjs ${{matrix.graphql_version}}
       - name: Use ESLint v${{matrix.eslint_version}}
         run: node scripts/match-eslint.mjs ${{matrix.eslint_version}}
       - name: Install Dependencies using Yarn

diff --git a/packages/plugin/src/rules/graphql-js-validation.ts b/packages/plugin/src/rules/graphql-js-validation.ts
@@ -2,15 +2,14 @@ import type { AST } from 'eslint';
 import type { JSONSchema4 } from 'json-schema';
 import {
   Kind,
-  TypeInfo,
   DocumentNode,
   GraphQLSchema,
   ValidationRule,
   FragmentDefinitionNode,
   OperationDefinitionNode,
   visit,
   validate,
-  visitWithTypeInfo,
+  ASTVisitor,
 } from 'graphql';
 import { validateSDL } from 'graphql/validation/validate';
 import type { GraphQLESLintRule, GraphQLESLintRuleContext } from '../types';
@@ -65,59 +64,47 @@ function validateDocument(
   }
 }
 
-type FragmentInfo = `${string}:${string}`;
+type GetFragmentDefsAndFragmentSpreads = {
+  fragmentDefs: Set<string>;
+  fragmentSpreads: Set<string>;
+};
 
-const getFragmentDefsAndFragmentSpreads = (
-  schema: GraphQLSchema,
-  node: DocumentNode
-): {
-  fragmentDefs: Set<FragmentInfo>;
-  fragmentSpreads: Set<FragmentInfo>;
-} => {
-  const typeInfo = new TypeInfo(schema);
-  const fragmentDefs = new Set<FragmentInfo>();
-  const fragmentSpreads = new Set<FragmentInfo>();
+const getFragmentDefsAndFragmentSpreads = (node: DocumentNode): GetFragmentDefsAndFragmentSpreads => {
+  const fragmentDefs = new Set<string>();
+  const fragmentSpreads = new Set<string>();
 
-  const visitor = visitWithTypeInfo(typeInfo, {
+  const visitor: ASTVisitor = {
     FragmentDefinition(node) {
-      fragmentDefs.add(`${node.name.value}:${node.typeCondition.name.value}`);
+      fragmentDefs.add(node.name.value);
     },
     FragmentSpread(node) {
-      const parentType = typeInfo.getParentType();
-      if (parentType) {
-        fragmentSpreads.add(`${node.name.value}:${parentType.name}`);
-      }
+      fragmentSpreads.add(node.name.value);
     },
-  });
+  };
 
   visit(node, visitor);
   return { fragmentDefs, fragmentSpreads };
 };
 
-const getMissingFragments = (schema: GraphQLSchema, node: DocumentNode): FragmentInfo[] => {
-  const { fragmentDefs, fragmentSpreads } = getFragmentDefsAndFragmentSpreads(schema, node);
+const getMissingFragments = (node: DocumentNode): string[] => {
+  const { fragmentDefs, fragmentSpreads } = getFragmentDefsAndFragmentSpreads(node);
   return [...fragmentSpreads].filter(name => !fragmentDefs.has(name));
 };
 
 type GetDocumentNode = (props: {
   ruleId: string;
   context: GraphQLESLintRuleContext;
-  schema: GraphQLSchema;
   node: DocumentNode;
 }) => DocumentNode;
 
-const handleMissingFragments: GetDocumentNode = ({ ruleId, context, schema, node }) => {
-  const missingFragments = getMissingFragments(schema, node);
+const handleMissingFragments: GetDocumentNode = ({ ruleId, context, node }) => {
+  const missingFragments = getMissingFragments(node);
   if (missingFragments.length > 0) {
     const siblings = requireSiblingsOperations(ruleId, context);
     const fragmentsToAdd: FragmentDefinitionNode[] = [];
 
-    for (const missingFragment of missingFragments) {
-      const [fragmentName, fragmentTypeName] = missingFragment.split(':');
-      const [foundFragment] = siblings
-        .getFragment(fragmentName)
-        .map(source => source.document)
-        .filter(fragment => fragment.typeCondition.name.value === fragmentTypeName);
+    for (const fragmentName of missingFragments) {
+      const [foundFragment] = siblings.getFragment(fragmentName).map(source => source.document);
       if (foundFragment) {
         fragmentsToAdd.push(foundFragment);
       }
@@ -128,7 +115,6 @@ const handleMissingFragments: GetDocumentNode = ({ ruleId, context, schema, node
       return handleMissingFragments({
         ruleId,
         context,
-        schema,
         node: {
           kind: Kind.DOCUMENT,
           definitions: [...node.definitions, ...fragmentsToAdd],
@@ -182,7 +168,7 @@ const validationToRule = (
             const schema = docs.requiresSchema ? requireGraphQLSchemaFromContext(ruleId, context) : null;
 
             const documentNode = getDocumentNode
-              ? getDocumentNode({ ruleId, context, schema, node: node.rawNode() })
+              ? getDocumentNode({ ruleId, context, node: node.rawNode() })
               : node.rawNode();
 
             validateDocument(context, schema, documentNode, ruleFn);
@@ -360,7 +346,7 @@ export const GRAPHQL_JS_VALIDATIONS: Record<string, GraphQLESLintRule> = Object.
       requiresSchema: true,
       requiresSiblings: true,
     },
-    ({ ruleId, context, schema, node }) => {
+    ({ ruleId, context, node }) => {
       const siblings = requireSiblingsOperations(ruleId, context);
       const FilePathToDocumentsMap = [...siblings.getOperations(), ...siblings.getFragments()].reduce<
         Record<string, (OperationDefinitionNode | FragmentDefinitionNode)[]>
@@ -371,15 +357,15 @@ export const GRAPHQL_JS_VALIDATIONS: Record<string, GraphQLESLintRule> = Object.
       }, Object.create(null));
 
       const getParentNode = (currentFilePath: string, node: DocumentNode): DocumentNode => {
-        const { fragmentDefs } = getFragmentDefsAndFragmentSpreads(schema, node);
+        const { fragmentDefs } = getFragmentDefsAndFragmentSpreads(node);
         if (fragmentDefs.size === 0) {
           return node;
         }
         // skip iteration over documents for current filepath
         delete FilePathToDocumentsMap[currentFilePath];
 
         for (const [filePath, documents] of Object.entries(FilePathToDocumentsMap)) {
-          const missingFragments = getMissingFragments(schema, {
+          const missingFragments = getMissingFragments({
             kind: Kind.DOCUMENT,
             definitions: documents,
           });

diff --git a/packages/plugin/tests/known-fragment-names.spec.ts b/packages/plugin/tests/known-fragment-names.spec.ts
@@ -39,6 +39,89 @@ ruleTester.runGraphQLTests('known-fragment-names', rules['known-fragment-names']
         ],
       },
     },
+    {
+      name: 'should work when interface implemented',
+      code: /* GraphQL */ `
+        fragment Introduction on Introduction {
+          introText {
+            ...ContentUnit
+          }
+        }
+      `,
+      parserOptions: {
+        schema: /* GraphQL */ `
+          interface ContentUnit {
+            contentSets: Int
+          }
+
+          type IntroText implements ContentUnit {
+            contentSets: Int
+          }
+
+          type Introduction {
+            introText: IntroText
+          }
+
+          type Query {
+            foo: Int
+          }
+        `,
+        operations: /* GraphQL */ `
+          fragment ContentUnit on ContentUnit {
+            contentSets {
+              id
+            }
+          }
+        `,
+      },
+    },
+    {
+      name: 'should work when with union',
+      code: /* GraphQL */ `
+        query {
+          animal {
+            ...AnimalFields
+          }
+        }
+      `,
+      parserOptions: {
+        schema: /* GraphQL */ `
+          type Cat {
+            name: String
+          }
+
+          type Dog {
+            age: String
+          }
+
+          union AnimalUnion = Cat | Dog
+
+          type Animal {
+            animal: AnimalUnion
+          }
+
+          type Query {
+            animal: Animal
+          }
+        `,
+        operations: /* GraphQL */ `
+          fragment CatFields on Cat {
+            title
+          }
+
+          fragment DogFields on Dog {
+            url
+          }
+
+          fragment AnimalFields on AnimalUnion {
+            animal {
+              ...CatFields
+              ...DogFields
+            }
+          }
+        `,
+      },
+    },
   ],
   invalid: [
     {

diff --git a/scripts/match-graphql.js → scripts/match-graphql.mjs b/scripts/match-graphql.js → scripts/match-graphql.mjs
@@ -1,18 +1,14 @@
-const { writeFileSync } = require('fs');
-const { resolve } = require('path');
-const { argv } = require('process');
+import { readFileSync, writeFileSync } from 'node:fs';
+import { resolve } from 'node:path';
 
-const pkgPath = resolve(__dirname, '../package.json');
+const pkgPath = resolve(process.cwd(), 'package.json');
+const version = process.argv[2];
+const pkg = JSON.parse(readFileSync(pkgPath));
 
-const pkg = require(pkgPath);
-
-const version = argv[2];
-
-pkg.resolutions = pkg.resolutions || {};
 if (pkg.resolutions.graphql.startsWith(version)) {
   // eslint-disable-next-line no-console
   console.info(`GraphQL v${version} is match! Skipping.`);
-  return;
+  process.exit();
 }
 
 const npmVersion = version.includes('-') ? version : `^${version}`;