From 05a4c2fe66a46b1d055c3d68518966ce1e9d2028 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 4 Apr 2021 21:18:44 +0300 Subject: [PATCH] Switch some of 'Object.keys' to more modern ES6 constructs --- flow-typed/core.js | 4 +-- src/jsutils/inspect.js | 12 ++++---- src/jsutils/mapValue.d.ts | 4 +-- src/jsutils/mapValue.js | 4 +-- src/language/__tests__/toJSONDeep.js | 7 ++--- src/utilities/lexicographicSortSchema.js | 11 ++++--- src/validation/rules/KnownTypeNamesRule.js | 7 +++-- .../rules/OverlappingFieldsCanBeMergedRule.js | 3 +- .../rules/PossibleTypeExtensionsRule.js | 8 ++--- .../rules/ProvidedRequiredArgumentsRule.js | 30 ++++++++----------- 10 files changed, 40 insertions(+), 50 deletions(-) diff --git a/flow-typed/core.js b/flow-typed/core.js index e7c0da6064..38e3f8f679 100644 --- 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(obj: { [key: string]: T, __proto__: null }): Array<[string, T]>; // graphql-js HACK + static entries(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(obj: { [key: string]: T, __proto__: null }): Array; // graphql-js HACK + static values(obj: { +[key: string]: T, __proto__: null }): Array; // graphql-js HACK static values(object: $NotNullOrVoid): Array; /** * Determines whether an object has a property with the specified name. diff --git a/src/jsutils/inspect.js b/src/jsutils/inspect.js index b4cebc4a8a..2c90dc4ca2 100644 --- a/src/jsutils/inspect.js +++ b/src/jsutils/inspect.js @@ -54,8 +54,8 @@ function formatObjectValue( } function formatObject(object: Object, seenValues: Array): string { - const keys = Object.keys(object); - if (keys.length === 0) { + const entries = Object.entries(object); + if (entries.length === 0) { return '{}'; } @@ -63,11 +63,9 @@ function formatObject(object: Object, seenValues: Array): string { 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 index b82ffedce9..31bb779e93 100644 --- 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( - map: ObjMap, + map: ReadOnlyObjMap, fn: (value: T, key: string) => V, ): ObjMap; diff --git a/src/jsutils/mapValue.js b/src/jsutils/mapValue.js index dee335b664..8bdc175baf 100644 --- 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( - map: ObjMap, + map: ReadOnlyObjMap, fn: (value: T, key: string) => V, ): ObjMap { const result = Object.create(null); diff --git a/src/language/__tests__/toJSONDeep.js b/src/language/__tests__/toJSONDeep.js index 71d543a068..abd4f50b84 100644 --- 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 index ea15603147..dc368426bf 100644 --- 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(map: ObjMap, sortValueFn?: (T) => R): ObjMap { +function sortObjMap(map: ObjMap, sortValueFn: (T) => R): ObjMap { 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 index bf443861a2..86046484d1 100644 --- 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 index 3468738427..a0e6773f73 100644 --- 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 index 0a47c7da1a..c342e20e87 100644 --- 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 index ddcc49f49c..0ddc8f784d 100644 --- 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, ), );