Replace all instances of ObjMap<boolean> with ES6 Set (#3688)

Motivation: I was inspecting code for unrelated reason and just spotted this pattern.
I think it worth to refactor it since it easier to understand intention looking on code that uses Set's has/add methods.
Benchmark shows no measurable changes in CPU/memory usages.

Author: IvanGoncharov

src/type/schema.ts

@@ -146,7 +146,11 @@ export class GraphQLSchema {
   private _subscriptionType: Maybe<GraphQLObjectType>;
   private _directives: ReadonlyArray<GraphQLDirective>;
   private _typeMap: TypeMap;
-  private _subTypeMap: ObjMap<ObjMap<boolean>>;
+  private _subTypeMap: Map<
+    GraphQLAbstractType,
+    Set<GraphQLObjectType | GraphQLInterfaceType>
+  >;
+
   private _implementationsMap: ObjMap<{
     objects: Array<GraphQLObjectType>;
     interfaces: Array<GraphQLInterfaceType>;
@@ -202,7 +206,7 @@ export class GraphQLSchema {
 
     // Storing the resulting map for reference by the schema.
     this._typeMap = Object.create(null);
-    this._subTypeMap = Object.create(null);
+    this._subTypeMap = new Map();
     // Keep track of all implementations by interface name.
     this._implementationsMap = Object.create(null);
 
@@ -308,27 +312,21 @@ export class GraphQLSchema {
     abstractType: GraphQLAbstractType,
     maybeSubType: GraphQLObjectType | GraphQLInterfaceType,
   ): boolean {
-    let map = this._subTypeMap[abstractType.name];
-    if (map === undefined) {
-      map = Object.create(null);
-
+    let set = this._subTypeMap.get(abstractType);
+    if (set === undefined) {
       if (isUnionType(abstractType)) {
-        for (const type of abstractType.getTypes()) {
-          map[type.name] = true;
-        }
+        set = new Set<GraphQLObjectType>(abstractType.getTypes());
       } else {
         const implementations = this.getImplementations(abstractType);
-        for (const type of implementations.objects) {
-          map[type.name] = true;
-        }
-        for (const type of implementations.interfaces) {
-          map[type.name] = true;
-        }
+        set = new Set<GraphQLObjectType | GraphQLInterfaceType>([
+          ...implementations.objects,
+          ...implementations.interfaces,
+        ]);
       }
 
-      this._subTypeMap[abstractType.name] = map;
+      this._subTypeMap.set(abstractType, set);
     }
-    return map[maybeSubType.name] !== undefined;
+    return set.has(maybeSubType);
   }
 
   getDirectives(): ReadonlyArray<GraphQLDirective> {

src/type/validate.ts

@@ -326,7 +326,7 @@ function validateInterfaces(
   context: SchemaValidationContext,
   type: GraphQLObjectType | GraphQLInterfaceType,
 ): void {
-  const ifaceTypeNames = Object.create(null);
+  const ifaceTypeNames = new Set<string>();
   for (const iface of type.getInterfaces()) {
     if (!isInterfaceType(iface)) {
       context.reportError(
@@ -345,15 +345,15 @@ function validateInterfaces(
       continue;
     }
 
-    if (ifaceTypeNames[iface.name]) {
+    if (ifaceTypeNames.has(iface.name)) {
       context.reportError(
         `Type ${type.name} can only implement ${iface.name} once.`,
         getAllImplementsInterfaceNodes(type, iface),
       );
       continue;
     }
 
-    ifaceTypeNames[iface.name] = true;
+    ifaceTypeNames.add(iface.name);
 
     validateTypeImplementsAncestors(context, type, iface);
     validateTypeImplementsInterface(context, type, iface);
@@ -470,16 +470,16 @@ function validateUnionMembers(
     );
   }
 
-  const includedTypeNames = Object.create(null);
+  const includedTypeNames = new Set<string>();
   for (const memberType of memberTypes) {
-    if (includedTypeNames[memberType.name]) {
+    if (includedTypeNames.has(memberType.name)) {
       context.reportError(
         `Union type ${union.name} can only include type ${memberType.name} once.`,
         getUnionMemberTypeNodes(union, memberType.name),
       );
       continue;
     }
-    includedTypeNames[memberType.name] = true;
+    includedTypeNames.add(memberType.name);
     if (!isObjectType(memberType)) {
       context.reportError(
         `Union type ${union.name} can only include Object types, ` +
@@ -551,7 +551,7 @@ function createInputObjectCircularRefsValidator(
   // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'.
   // Tracks already visited types to maintain O(N) and to ensure that cycles
   // are not redundantly reported.
-  const visitedTypes = Object.create(null);
+  const visitedTypes = new Set<GraphQLInputObjectType>();
 
   // Array of types nodes used to produce meaningful errors
   const fieldPath: Array<GraphQLInputField> = [];
@@ -565,11 +565,11 @@ function createInputObjectCircularRefsValidator(
   // It does not terminate when a cycle was found but continues to explore
   // the graph to find all possible cycles.
   function detectCycleRecursive(inputObj: GraphQLInputObjectType): void {
-    if (visitedTypes[inputObj.name]) {
+    if (visitedTypes.has(inputObj)) {
       return;
     }
 
-    visitedTypes[inputObj.name] = true;
+    visitedTypes.add(inputObj);
     fieldPathIndexByTypeName[inputObj.name] = fieldPath.length;
 
     const fields = Object.values(inputObj.getFields());

src/validation/ValidationContext.ts

@@ -114,14 +114,14 @@ export class ASTValidationContext {
     let fragments = this._recursivelyReferencedFragments.get(operation);
     if (!fragments) {
       fragments = [];
-      const collectedNames = Object.create(null);
+      const collectedNames = new Set<string>();
       const nodesToVisit: Array<SelectionSetNode> = [operation.selectionSet];
       let node: SelectionSetNode | undefined;
       while ((node = nodesToVisit.pop())) {
         for (const spread of this.getFragmentSpreads(node)) {
           const fragName = spread.name.value;
-          if (collectedNames[fragName] !== true) {
-            collectedNames[fragName] = true;
+          if (!collectedNames.has(fragName)) {
+            collectedNames.add(fragName);
             const fragment = this.getFragment(fragName);
             if (fragment) {
               fragments.push(fragment);

src/validation/rules/KnownTypeNamesRule.ts

@@ -30,34 +30,27 @@ import type {
 export function KnownTypeNamesRule(
   context: ValidationContext | SDLValidationContext,
 ): ASTVisitor {
-  const schema = context.getSchema();
-  const existingTypesMap = schema ? schema.getTypeMap() : Object.create(null);
+  const { definitions } = context.getDocument();
+  const existingTypesMap = context.getSchema()?.getTypeMap() ?? {};
 
-  const definedTypes = Object.create(null);
-  for (const def of context.getDocument().definitions) {
-    if (isTypeDefinitionNode(def)) {
-      definedTypes[def.name.value] = true;
-    }
-  }
-
-  const typeNames = [
+  const typeNames = new Set([
     ...Object.keys(existingTypesMap),
-    ...Object.keys(definedTypes),
-  ];
+    ...definitions.filter(isTypeDefinitionNode).map((def) => def.name.value),
+  ]);
 
   return {
     NamedType(node, _1, parent, _2, ancestors) {
       const typeName = node.name.value;
-      if (!existingTypesMap[typeName] && !definedTypes[typeName]) {
+      if (!typeNames.has(typeName)) {
         const definitionNode = ancestors[2] ?? parent;
         const isSDL = definitionNode != null && isSDLNode(definitionNode);
-        if (isSDL && standardTypeNames.includes(typeName)) {
+        if (isSDL && standardTypeNames.has(typeName)) {
           return;
         }
 
         const suggestedTypes = suggestionList(
           typeName,
-          isSDL ? standardTypeNames.concat(typeNames) : typeNames,
+          isSDL ? [...standardTypeNames, ...typeNames] : [...typeNames],
         );
         context.reportError(
           new GraphQLError(
@@ -70,8 +63,8 @@ export function KnownTypeNamesRule(
   };
 }
 
-const standardTypeNames = [...specifiedScalarTypes, ...introspectionTypes].map(
-  (type) => type.name,
+const standardTypeNames = new Set<string>(
+  [...specifiedScalarTypes, ...introspectionTypes].map((type) => type.name),
 );
 
 function isSDLNode(value: ASTNode | ReadonlyArray<ASTNode>): boolean {

src/validation/rules/NoFragmentCyclesRule.ts

@@ -23,7 +23,7 @@ export function NoFragmentCyclesRule(
 ): ASTVisitor {
   // Tracks already visited fragments to maintain O(N) and to ensure that cycles
   // are not redundantly reported.
-  const visitedFrags: ObjMap<boolean> = Object.create(null);
+  const visitedFrags = new Set<string>();
 
   // Array of AST nodes used to produce meaningful errors
   const spreadPath: Array<FragmentSpreadNode> = [];
@@ -43,12 +43,12 @@ export function NoFragmentCyclesRule(
   // It does not terminate when a cycle was found but continues to explore
   // the graph to find all possible cycles.
   function detectCycleRecursive(fragment: FragmentDefinitionNode): void {
-    if (visitedFrags[fragment.name.value]) {
+    if (visitedFrags.has(fragment.name.value)) {
       return;
     }
 
     const fragmentName = fragment.name.value;
-    visitedFrags[fragmentName] = true;
+    visitedFrags.add(fragmentName);
 
     const spreadNodes = context.getFragmentSpreads(fragment.selectionSet);
     if (spreadNodes.length === 0) {

src/validation/rules/NoUndefinedVariablesRule.ts

@@ -15,33 +15,26 @@ import type { ValidationContext } from '../ValidationContext';
 export function NoUndefinedVariablesRule(
   context: ValidationContext,
 ): ASTVisitor {
-  let variableNameDefined = Object.create(null);
-
   return {
-    OperationDefinition: {
-      enter() {
-        variableNameDefined = Object.create(null);
-      },
-      leave(operation) {
-        const usages = context.getRecursiveVariableUsages(operation);
+    OperationDefinition(operation) {
+      const variableNameDefined = new Set<string>(
+        operation.variableDefinitions?.map((node) => node.variable.name.value),
+      );
 
-        for (const { node } of usages) {
-          const varName = node.name.value;
-          if (variableNameDefined[varName] !== true) {
-            context.reportError(
-              new GraphQLError(
-                operation.name
-                  ? `Variable "$${varName}" is not defined by operation "${operation.name.value}".`
-                  : `Variable "$${varName}" is not defined.`,
-                { nodes: [node, operation] },
-              ),
-            );
-          }
+      const usages = context.getRecursiveVariableUsages(operation);
+      for (const { node } of usages) {
+        const varName = node.name.value;
+        if (!variableNameDefined.has(varName)) {
+          context.reportError(
+            new GraphQLError(
+              operation.name
+                ? `Variable "$${varName}" is not defined by operation "${operation.name.value}".`
+                : `Variable "$${varName}" is not defined.`,
+              { nodes: [node, operation] },
+            ),
+          );
         }
-      },
-    },
-    VariableDefinition(node) {
-      variableNameDefined[node.variable.name.value] = true;
+      }
     },
   };
 }

src/validation/rules/NoUnusedFragmentsRule.ts

@@ -1,9 +1,6 @@
 import { GraphQLError } from '../../error/GraphQLError';
 
-import type {
-  FragmentDefinitionNode,
-  OperationDefinitionNode,
-} from '../../language/ast';
+import type { FragmentDefinitionNode } from '../../language/ast';
 import type { ASTVisitor } from '../../language/visitor';
 
 import type { ASTValidationContext } from '../ValidationContext';
@@ -19,12 +16,16 @@ import type { ASTValidationContext } from '../ValidationContext';
 export function NoUnusedFragmentsRule(
   context: ASTValidationContext,
 ): ASTVisitor {
-  const operationDefs: Array<OperationDefinitionNode> = [];
+  const fragmentNameUsed = new Set<string>();
   const fragmentDefs: Array<FragmentDefinitionNode> = [];
 
   return {
-    OperationDefinition(node) {
-      operationDefs.push(node);
+    OperationDefinition(operation) {
+      for (const fragment of context.getRecursivelyReferencedFragments(
+        operation,
+      )) {
+        fragmentNameUsed.add(fragment.name.value);
+      }
       return false;
     },
     FragmentDefinition(node) {
@@ -33,18 +34,9 @@ export function NoUnusedFragmentsRule(
     },
     Document: {
       leave() {
-        const fragmentNameUsed = Object.create(null);
-        for (const operation of operationDefs) {
-          for (const fragment of context.getRecursivelyReferencedFragments(
-            operation,
-          )) {
-            fragmentNameUsed[fragment.name.value] = true;
-          }
-        }
-
         for (const fragmentDef of fragmentDefs) {
           const fragName = fragmentDef.name.value;
-          if (fragmentNameUsed[fragName] !== true) {
+          if (!fragmentNameUsed.has(fragName)) {
             context.reportError(
               new GraphQLError(`Fragment "${fragName}" is never used.`, {
                 nodes: fragmentDef,

src/validation/rules/NoUnusedVariablesRule.ts

@@ -1,6 +1,5 @@
 import { GraphQLError } from '../../error/GraphQLError';
 
-import type { VariableDefinitionNode } from '../../language/ast';
 import type { ASTVisitor } from '../../language/visitor';
 
 import type { ValidationContext } from '../ValidationContext';
@@ -14,38 +13,29 @@ import type { ValidationContext } from '../ValidationContext';
  * See https://spec.graphql.org/draft/#sec-All-Variables-Used
  */
 export function NoUnusedVariablesRule(context: ValidationContext): ASTVisitor {
-  let variableDefs: Array<VariableDefinitionNode> = [];
-
   return {
-    OperationDefinition: {
-      enter() {
-        variableDefs = [];
-      },
-      leave(operation) {
-        const variableNameUsed = Object.create(null);
-        const usages = context.getRecursiveVariableUsages(operation);
-
-        for (const { node } of usages) {
-          variableNameUsed[node.name.value] = true;
-        }
+    OperationDefinition(operation) {
+      const usages = context.getRecursiveVariableUsages(operation);
+      const variableNameUsed = new Set<string>(
+        usages.map(({ node }) => node.name.value),
+      );
 
-        for (const variableDef of variableDefs) {
-          const variableName = variableDef.variable.name.value;
-          if (variableNameUsed[variableName] !== true) {
-            context.reportError(
-              new GraphQLError(
-                operation.name
-                  ? `Variable "$${variableName}" is never used in operation "${operation.name.value}".`
-                  : `Variable "$${variableName}" is never used.`,
-                { nodes: variableDef },
-              ),
-            );
-          }
+      // FIXME: https://github.com/graphql/graphql-js/issues/2203
+      /* c8 ignore next */
+      const variableDefinitions = operation.variableDefinitions ?? [];
+      for (const variableDef of variableDefinitions) {
+        const variableName = variableDef.variable.name.value;
+        if (!variableNameUsed.has(variableName)) {
+          context.reportError(
+            new GraphQLError(
+              operation.name
+                ? `Variable "$${variableName}" is never used in operation "${operation.name.value}".`
+                : `Variable "$${variableName}" is never used.`,
+              { nodes: variableDef },
+            ),
+          );
         }
-      },
-    },
-    VariableDefinition(def) {
-      variableDefs.push(def);
+      }
     },
   };
 }