2️⃣ Fix caching for no-unreachable-types and no-unused-fields rules (#548)

Author: Dimitri POSTOLOV

.changeset/lemon-donkeys-hammer.md

@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix caching for `no-unreachable-types` and `no-unused-fields` rules

packages/plugin/src/graphql-ast.ts

@@ -9,46 +9,41 @@ import {
   GraphQLNamedType,
   GraphQLInterfaceType,
   GraphQLArgument,
+  GraphQLDirective,
   isObjectType,
   isInterfaceType,
   isUnionType,
   isInputObjectType,
   isListType,
   isNonNullType,
-  GraphQLDirective,
   TypeInfo,
   visit,
   visitWithTypeInfo,
 } from 'graphql';
 import { SiblingOperations } from './sibling-operations';
 
-export function createReachableTypesService(schema: GraphQLSchema): () => Set<string>;
-export function createReachableTypesService(schema?: GraphQLSchema): () => Set<string> | null {
-  if (schema) {
-    let cache: Set<string> = null;
-    return () => {
-      if (!cache) {
-        cache = collectReachableTypes(schema);
-      }
+export type ReachableTypes = Set<string>;
 
-      return cache;
-    };
-  }
+let reachableTypesCache: ReachableTypes;
 
-  return () => null;
-}
+export function getReachableTypes(schema: GraphQLSchema): ReachableTypes {
+  // We don't want cache reachableTypes on test environment
+  // Otherwise reachableTypes will be same for all tests
+  if (process.env.NODE_ENV !== 'test' && reachableTypesCache) {
+    return reachableTypesCache
+  }
 
-export function collectReachableTypes(schema: GraphQLSchema): Set<string> {
-  const reachableTypes = new Set<string>();
+  const reachableTypes: ReachableTypes = new Set();
 
   collectFromDirectives(schema.getDirectives());
   collectFrom(schema.getQueryType());
   collectFrom(schema.getMutationType());
   collectFrom(schema.getSubscriptionType());
 
-  return reachableTypes;
+  reachableTypesCache = reachableTypes;
+  return reachableTypesCache;
 
-  function collectFromDirectives(directives: readonly GraphQLDirective[]) {
+  function collectFromDirectives(directives: readonly GraphQLDirective[]): void {
     for (const directive of directives || []) {
       reachableTypes.add(directive.name);
       directive.args.forEach(collectFromArgument);
@@ -119,43 +114,33 @@ export function collectReachableTypes(schema: GraphQLSchema): Set<string> {
     if (isListType(type) || isNonNullType(type)) {
       return resolveName(type.ofType);
     }
-
     return type.name;
   }
 
   function shouldCollect(name: string): boolean {
-    if (!reachableTypes.has(name)) {
-      reachableTypes.add(name);
-      return true;
+    if (reachableTypes.has(name)) {
+      return false;
     }
-
-    return false;
+    reachableTypes.add(name);
+    return true;
   }
 }
 
-export type FieldsCache = Record<string, Set<string>>;
+export type UsedFields = Record<string, Set<string>>;
 
-export function createUsedFieldsService(schema: GraphQLSchema, operations: SiblingOperations): () => FieldsCache | null {
-  if (!schema || !operations) {
-    return () => null;
-  }
-
-  let cache: FieldsCache = null;
-
-  return () => {
-    if (!cache) {
-      cache = collectUsedFields(schema, operations);
-    }
+let usedFieldsCache: UsedFields;
 
-    return cache;
-  };
-}
+export function getUsedFields(schema: GraphQLSchema, operations: SiblingOperations): UsedFields {
+  // We don't want cache usedFields on test environment
+  // Otherwise usedFields will be same for all tests
+  if (process.env.NODE_ENV !== 'test' && usedFieldsCache) {
+    return usedFieldsCache;
+  }
 
-export function collectUsedFields(schema: GraphQLSchema, operations: SiblingOperations): FieldsCache {
-  const cache: FieldsCache = {};
+  const usedFields: UsedFields = {};
 
   const addField = (typeName, fieldName) => {
-    const fieldType = cache[typeName] ?? (cache[typeName] = new Set());
+    const fieldType = usedFields[typeName] ?? (usedFields[typeName] = new Set());
     fieldType.add(fieldName);
   };
 
@@ -180,14 +165,12 @@ export function collectUsedFields(schema: GraphQLSchema, operations: SiblingOper
     },
   });
 
-  const allDocuments = [
-    ...operations.getOperations(),
-    ...operations.getFragments(),
-  ];
+  const allDocuments = [...operations.getOperations(), ...operations.getFragments()];
 
   for (const { document } of allDocuments) {
     visit(document, visitor);
   }
 
-  return cache;
+  usedFieldsCache = usedFields;
+  return usedFieldsCache;
 }

packages/plugin/src/graphql-config.ts

@@ -23,14 +23,15 @@ export function loadGraphqlConfig(options: ParserOptions): GraphQLConfig {
     onDiskConfig ||
     new GraphQLConfig(
       {
-        config: {
-          schema: options.schema || '', // if undefined will throw error `Project 'default' not found`
-          documents: options.documents || options.operations,
-          extensions: options.extensions,
-          include: options.include,
-          exclude: options.exclude,
-          projects: options.projects,
-        },
+        config: options.projects
+          ? { projects: options.projects }
+          : {
+              schema: options.schema || '', // if `options.schema` is `undefined` will throw error `Project 'default' not found`
+              documents: options.documents || options.operations,
+              extensions: options.extensions,
+              include: options.include,
+              exclude: options.exclude,
+            },
         filepath: 'virtual-config',
       },
       [addCodeFileLoaderExtension]

packages/plugin/src/parser.ts

@@ -2,27 +2,26 @@ import { parseGraphQLSDL } from '@graphql-tools/utils';
 import { GraphQLError, TypeInfo } from 'graphql';
 import { Linter } from 'eslint';
 import { convertToESTree } from './estree-parser';
-import { GraphQLESLintParseResult, ParserOptions } from './types';
+import { GraphQLESLintParseResult, ParserOptions, ParserServices } from './types';
 import { extractTokens } from './utils';
 import { getSchema } from './schema';
 import { getSiblingOperations } from './sibling-operations';
 import { loadGraphqlConfig } from './graphql-config';
-import { createReachableTypesService, createUsedFieldsService } from './graphql-ast';
+import { getReachableTypes, getUsedFields } from './graphql-ast';
 
 export function parse(code: string, options?: ParserOptions): Linter.ESLintParseResult['ast'] {
   return parseForESLint(code, options).ast;
 }
 
-export function parseForESLint(code: string, options?: ParserOptions): GraphQLESLintParseResult {
+export function parseForESLint(code: string, options: ParserOptions = {}): GraphQLESLintParseResult {
   const gqlConfig = loadGraphqlConfig(options);
   const schema = getSchema(options, gqlConfig);
-  const siblingOperations = getSiblingOperations(options, gqlConfig);
-  const parserServices = {
+  const parserServices: ParserServices = {
     hasTypeInfo: schema !== null,
     schema,
-    siblingOperations,
-    getReachableTypes: createReachableTypesService(schema),
-    getUsedFields: createUsedFieldsService(schema, siblingOperations),
+    siblingOperations: getSiblingOperations(options, gqlConfig),
+    reachableTypes: getReachableTypes,
+    usedFields: getUsedFields,
   };
 
   try {

packages/plugin/src/sibling-operations.ts

@@ -5,6 +5,7 @@ import {
   OperationDefinitionNode,
   SelectionSetNode,
   visit,
+  OperationTypeNode,
 } from 'graphql';
 import { Source, asArray } from '@graphql-tools/utils';
 import { GraphQLConfig } from 'graphql-config';
@@ -25,7 +26,7 @@ export type SiblingOperations = {
     recursive: boolean
   ): FragmentDefinitionNode[];
   getOperation(operationName: string): OperationSource[];
-  getOperationByType(operationType: 'query' | 'mutation' | 'subscription'): OperationSource[];
+  getOperationByType(operationType: OperationTypeNode): OperationSource[];
 };
 
 const operationsCache: Map<string, Source[]> = new Map();
@@ -106,10 +107,8 @@ export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLC
           }
         }
       }
-
       fragmentsCache = result;
     }
-
     return fragmentsCache;
   };
 
@@ -129,10 +128,8 @@ export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLC
           }
         }
       }
-
       cachedOperations = result;
     }
-
     return cachedOperations;
   };
 
@@ -144,7 +141,7 @@ export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLC
     collected: Map<string, FragmentDefinitionNode> = new Map()
   ) => {
     visit(selectable, {
-      FragmentSpread: (spread: FragmentSpreadNode) => {
+      FragmentSpread(spread: FragmentSpreadNode) {
         const name = spread.name.value;
         const fragmentInfo = getFragment(name);
 
@@ -153,32 +150,29 @@ export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLC
           console.warn(
             `Unable to locate fragment named "${name}", please make sure it's loaded using "parserOptions.operations"`
           );
-        } else {
-          const fragment = fragmentInfo[0];
-          const alreadyVisited = collected.has(name);
-
-          if (!alreadyVisited) {
-            collected.set(spread.name.value, fragment.document);
+          return;
+        }
+        const fragment = fragmentInfo[0];
+        const alreadyVisited = collected.has(name);
 
-            if (recursive) {
-              collectFragments(fragment.document, recursive, collected);
-            }
+        if (!alreadyVisited) {
+          collected.set(name, fragment.document);
+          if (recursive) {
+            collectFragments(fragment.document, recursive, collected);
           }
         }
       },
     });
-
     return collected;
   };
 
-  const siblingOperations = {
+  const siblingOperations: SiblingOperations = {
     available: true,
     getFragments,
     getOperations,
     getFragment,
-    getFragmentByType: (typeName: string) =>
-      getFragments().filter(f => f.document.typeCondition?.name?.value === typeName),
-    getOperation: (name: string) => getOperations().filter(o => o.document.name?.value === name),
+    getFragmentByType: typeName => getFragments().filter(f => f.document.typeCondition?.name?.value === typeName),
+    getOperation: name => getOperations().filter(o => o.document.name?.value === name),
     getOperationByType: type => getOperations().filter(o => o.document.operation === type),
     getFragmentsInUse: (selectable, recursive = true) => Array.from(collectFragments(selectable, recursive).values()),
   };

packages/plugin/src/types.ts

@@ -5,7 +5,7 @@ import { IExtensions, IGraphQLProject, DocumentPointer, SchemaPointer } from 'gr
 import { GraphQLESLintRuleListener } from './testkit';
 import { GraphQLESTreeNode } from './estree-parser';
 import { SiblingOperations } from './sibling-operations';
-import { FieldsCache } from './graphql-ast';
+import { getReachableTypes, getUsedFields } from './graphql-ast';
 
 export interface ParserOptions {
   schema?: SchemaPointer;
@@ -22,11 +22,11 @@ export interface ParserOptions {
 }
 
 export type ParserServices = {
-  siblingOperations: SiblingOperations;
   hasTypeInfo: boolean;
   schema: GraphQLSchema | null;
-  getReachableTypes: () => Set<string> | null;
-  getUsedFields: () => FieldsCache | null;
+  siblingOperations: SiblingOperations;
+  reachableTypes: typeof getReachableTypes;
+  usedFields: typeof getUsedFields;
 };
 
 export type GraphQLESLintParseResult = Linter.ESLintParseResult & {

packages/plugin/src/utils.ts

@@ -4,10 +4,13 @@ import { Source, Lexer, GraphQLSchema, Token, DocumentNode } from 'graphql';
 import { GraphQLESLintRuleContext } from './types';
 import { AST } from 'eslint';
 import { SiblingOperations } from './sibling-operations';
-import { FieldsCache } from './graphql-ast';
+import { UsedFields, ReachableTypes } from './graphql-ast';
 
-export function requireSiblingsOperations(ruleName: string, context: GraphQLESLintRuleContext<any>): SiblingOperations {
-  if (!context || !context.parserServices) {
+export function requireSiblingsOperations(
+  ruleName: string,
+  context: GraphQLESLintRuleContext
+): SiblingOperations | never {
+  if (!context.parserServices) {
     throw new Error(
       `Rule '${ruleName}' requires 'parserOptions.operations' to be set and loaded. See http://bit.ly/graphql-eslint-operations for more info`
     );
@@ -24,9 +27,9 @@ export function requireSiblingsOperations(ruleName: string, context: GraphQLESLi
 
 export function requireGraphQLSchemaFromContext(
   ruleName: string,
-  context: GraphQLESLintRuleContext<any>
-): GraphQLSchema {
-  if (!context || !context.parserServices) {
+  context: GraphQLESLintRuleContext
+): GraphQLSchema | never {
+  if (!context.parserServices) {
     throw new Error(
       `Rule '${ruleName}' requires 'parserOptions.schema' to be set. See http://bit.ly/graphql-eslint-schema for more info`
     );
@@ -43,37 +46,16 @@ export function requireGraphQLSchemaFromContext(
 
 export function requireReachableTypesFromContext(
   ruleName: string,
-  context: GraphQLESLintRuleContext<any>
-): Set<string> {
-  if (!context || !context.parserServices) {
-    throw new Error(
-      `Rule '${ruleName}' requires 'parserOptions.schema' to be set. See http://bit.ly/graphql-eslint-schema for more info`
-    );
-  }
-
-  if (!context.parserServices.schema) {
-    throw new Error(
-      `Rule '${ruleName}' requires 'parserOptions.schema' to be set and schema to be loaded. See http://bit.ly/graphql-eslint-schema for more info`
-    );
-  }
-
-  return context.parserServices.getReachableTypes();
+  context: GraphQLESLintRuleContext
+): ReachableTypes | never {
+  const schema = requireGraphQLSchemaFromContext(ruleName, context);
+  return context.parserServices.reachableTypes(schema);
 }
 
-export function requireUsedFieldsFromContext(ruleName: string, context: GraphQLESLintRuleContext<any>): FieldsCache {
-  if (!context || !context.parserServices) {
-    throw new Error(
-      `Rule '${ruleName}' requires 'parserOptions.schema' to be set. See http://bit.ly/graphql-eslint-schema for more info`
-    );
-  }
-
-  if (!context.parserServices.schema) {
-    throw new Error(
-      `Rule '${ruleName}' requires 'parserOptions.schema' to be set and schema to be loaded. See http://bit.ly/graphql-eslint-schema for more info`
-    );
-  }
-
-  return context.parserServices.getUsedFields();
+export function requireUsedFieldsFromContext(ruleName: string, context: GraphQLESLintRuleContext): UsedFields | never {
+  const schema = requireGraphQLSchemaFromContext(ruleName, context);
+  const siblings = requireSiblingsOperations(ruleName, context);
+  return context.parserServices.usedFields(schema, siblings);
 }
 
 function getLexer(source: Source): Lexer {