Switch some of 'Object.keys' to more modern ES6 constructs (#3026)

graphql · Apr 4, 2021 · 179e479 · 179e479
1 parent 954913b
commit 179e479
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 50 deletions.
diff --git a/flow-typed/core.js b/flow-typed/core.js
@@ -128,7 +128,7 @@ declare class Object {
      * Returns an array of key/values of the enumerable properties of an object
      * @param object Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
      */
-    static entries<T>(obj: { [key: string]: T, __proto__: null }): Array<[string, T]>; // graphql-js HACK
+    static entries<T>(obj: { +[key: string]: T, __proto__: null }): Array<[string, T]>; // graphql-js HACK
     static entries(object: $NotNullOrVoid): Array<[string, mixed]>;
     /**
      * Prevents the modification of existing property attributes and values, and prevents the addition of new properties.
@@ -220,7 +220,7 @@ declare class Object {
      * Returns an array of values of the enumerable properties of an object
      * @param object Object that contains the properties and methods. This can be an object that you created or an existing Document Object Model (DOM) object.
      */
-    static values<T>(obj: { [key: string]: T, __proto__: null }): Array<T>; // graphql-js HACK
+    static values<T>(obj: { +[key: string]: T, __proto__: null }): Array<T>; // graphql-js HACK
     static values(object: $NotNullOrVoid): Array<mixed>;
     /**
      * Determines whether an object has a property with the specified name.

diff --git a/src/jsutils/inspect.js b/src/jsutils/inspect.js
@@ -54,20 +54,18 @@ function formatObjectValue(
 }
 
 function formatObject(object: Object, seenValues: Array<mixed>): string {
-  const keys = Object.keys(object);
-  if (keys.length === 0) {
+  const entries = Object.entries(object);
+  if (entries.length === 0) {
     return '{}';
   }
 
   if (seenValues.length > MAX_RECURSIVE_DEPTH) {
     return '[' + getObjectTag(object) + ']';
   }
 
-  const properties = keys.map((key) => {
-    const value = formatValue(object[key], seenValues);
-    return key + ': ' + value;
-  });
-
+  const properties = entries.map(
+    ([key, value]) => key + ': ' + formatValue(value, seenValues),
+  );
   return '{ ' + properties.join(', ') + ' }';
 }
 

diff --git a/src/jsutils/mapValue.d.ts b/src/jsutils/mapValue.d.ts
@@ -1,9 +1,9 @@
-import type { ObjMap } from './ObjMap';
+import type { ObjMap, ReadOnlyObjMap } from './ObjMap';
 /**
  * Creates an object map with the same keys as `map` and values generated by
  * running each value of `map` thru `fn`.
  */
 export function mapValue<T, V>(
-  map: ObjMap<T>,
+  map: ReadOnlyObjMap<T>,
   fn: (value: T, key: string) => V,
 ): ObjMap<V>;
diff --git a/src/jsutils/mapValue.js b/src/jsutils/mapValue.js
@@ -1,11 +1,11 @@
-import type { ObjMap } from './ObjMap';
+import type { ObjMap, ReadOnlyObjMap } from './ObjMap';
 
 /**
  * Creates an object map with the same keys as `map` and values generated by
  * running each value of `map` thru `fn`.
  */
 export function mapValue<T, V>(
-  map: ObjMap<T>,
+  map: ReadOnlyObjMap<T>,
   fn: (value: T, key: string) => V,
 ): ObjMap<V> {
   const result = Object.create(null);

diff --git a/src/language/__tests__/toJSONDeep.js b/src/language/__tests__/toJSONDeep.js
@@ -1,3 +1,4 @@
+import { mapValue } from '../../jsutils/mapValue';
 import { isObjectLike } from '../../jsutils/isObjectLike';
 
 /**
@@ -18,9 +19,5 @@ export function toJSONDeep(value: mixed): mixed {
     return value.map(toJSONDeep);
   }
 
-  const result = Object.create(null);
-  for (const prop of Object.keys(value)) {
-    result[prop] = toJSONDeep(value[prop]);
-  }
-  return result;
+  return mapValue(value, toJSONDeep);
 }
diff --git a/src/utilities/lexicographicSortSchema.js b/src/utilities/lexicographicSortSchema.js
@@ -139,7 +139,7 @@ export function lexicographicSortSchema(schema: GraphQLSchema): GraphQLSchema {
       const config = type.toConfig();
       return new GraphQLEnumType({
         ...config,
-        values: sortObjMap(config.values),
+        values: sortObjMap(config.values, (value) => value),
       });
     }
     // istanbul ignore else (See: 'https://github.com/graphql/graphql-js/issues/2618')
@@ -156,12 +156,11 @@ export function lexicographicSortSchema(schema: GraphQLSchema): GraphQLSchema {
   }
 }
 
-function sortObjMap<T, R>(map: ObjMap<T>, sortValueFn?: (T) => R): ObjMap<R> {
+function sortObjMap<T, R>(map: ObjMap<T>, sortValueFn: (T) => R): ObjMap<R> {
   const sortedMap = Object.create(null);
-  const sortedKeys = sortBy(Object.keys(map), (x) => x);
-  for (const key of sortedKeys) {
-    const value = map[key];
-    sortedMap[key] = sortValueFn ? sortValueFn(value) : value;
+  const sortedEntries = sortBy(Object.entries(map), ([key]) => key);
+  for (const [key, value] of sortedEntries) {
+    sortedMap[key] = sortValueFn(value);
   }
   return sortedMap;
 }

diff --git a/src/validation/rules/KnownTypeNamesRule.js b/src/validation/rules/KnownTypeNamesRule.js
@@ -38,9 +38,10 @@ export function KnownTypeNamesRule(
     }
   }
 
-  const typeNames = Object.keys(existingTypesMap).concat(
-    Object.keys(definedTypes),
-  );
+  const typeNames = [
+    ...Object.keys(existingTypesMap),
+    ...Object.keys(definedTypes),
+  ];
 
   return {
     NamedType(node, _1, parent, _2, ancestors) {

diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.js b/src/validation/rules/OverlappingFieldsCanBeMergedRule.js
@@ -505,10 +505,9 @@ function collectConflictsBetween(
   // response name. For any response name which appears in both provided field
   // maps, each field from the first field map must be compared to every field
   // in the second field map to find potential conflicts.
-  for (const responseName of Object.keys(fieldMap1)) {
+  for (const [responseName, fields1] of Object.entries(fieldMap1)) {
     const fields2 = fieldMap2[responseName];
     if (fields2) {
-      const fields1 = fieldMap1[responseName];
       for (let i = 0; i < fields1.length; i++) {
         for (let j = 0; j < fields2.length; j++) {
           const conflict = findConflict(

diff --git a/src/validation/rules/PossibleTypeExtensionsRule.js b/src/validation/rules/PossibleTypeExtensionsRule.js
@@ -72,10 +72,10 @@ export function PossibleTypeExtensionsRule(
         );
       }
     } else {
-      let allTypeNames = Object.keys(definedTypes);
-      if (schema) {
-        allTypeNames = allTypeNames.concat(Object.keys(schema.getTypeMap()));
-      }
+      const allTypeNames = Object.keys({
+        ...definedTypes,
+        ...schema?.getTypeMap(),
+      });
 
       const suggestedTypes = suggestionList(typeName, allTypeNames);
       context.reportError(

diff --git a/src/validation/rules/ProvidedRequiredArgumentsRule.js b/src/validation/rules/ProvidedRequiredArgumentsRule.js
@@ -36,12 +36,12 @@ export function ProvidedRequiredArgumentsRule(
           return false;
         }
 
-        // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
-        const argNodes = fieldNode.arguments ?? [];
-        const argNodeMap = keyMap(argNodes, (arg) => arg.name.value);
+        const providedArgs = new Set(
+          // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
+          fieldNode.arguments?.map((arg) => arg.name.value),
+        );
         for (const argDef of fieldDef.args) {
-          const argNode = argNodeMap[argDef.name];
-          if (!argNode && isRequiredArgument(argDef)) {
+          if (!providedArgs.has(argDef.name) && isRequiredArgument(argDef)) {
             const argTypeStr = inspect(argDef.type);
             context.reportError(
               new GraphQLError(
@@ -65,9 +65,7 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
   const requiredArgsMap = Object.create(null);
 
   const schema = context.getSchema();
-  const definedDirectives = schema
-    ? schema.getDirectives()
-    : specifiedDirectives;
+  const definedDirectives = schema?.getDirectives() ?? specifiedDirectives;
   for (const directive of definedDirectives) {
     requiredArgsMap[directive.name] = keyMap(
       directive.args.filter(isRequiredArgument),
@@ -97,17 +95,15 @@ export function ProvidedRequiredArgumentsOnDirectivesRule(
         if (requiredArgs) {
           // istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
           const argNodes = directiveNode.arguments ?? [];
-          const argNodeMap = keyMap(argNodes, (arg) => arg.name.value);
-          for (const argName of Object.keys(requiredArgs)) {
-            if (!argNodeMap[argName]) {
-              const argType = requiredArgs[argName].type;
-              const argTypeStr = isType(argType)
-                ? inspect(argType)
-                : print(argType);
-
+          const argNodeMap = new Set(argNodes.map((arg) => arg.name.value));
+          for (const [argName, argDef] of Object.entries(requiredArgs)) {
+            if (!argNodeMap.has(argName)) {
+              const argType = isType(argDef.type)
+                ? inspect(argDef.type)
+                : print(argDef.type);
               context.reportError(
                 new GraphQLError(
-                  `Directive "@${directiveName}" argument "${argName}" of type "${argTypeStr}" is required, but it was not provided.`,
+                  `Directive "@${directiveName}" argument "${argName}" of type "${argType}" is required, but it was not provided.`,
                   directiveNode,
                 ),
               );