Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Apr 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions flow-typed/core.js
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 5 additions & 7 deletions src/jsutils/inspect.js
Expand Up @@ -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(', ') + ' }';
}

Expand Down
4 changes: 2 additions & 2 deletions 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>;
4 changes: 2 additions & 2 deletions 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);
Expand Down
7 changes: 2 additions & 5 deletions src/language/__tests__/toJSONDeep.js
@@ -1,3 +1,4 @@
import { mapValue } from '../../jsutils/mapValue';
import { isObjectLike } from '../../jsutils/isObjectLike';

/**
Expand All @@ -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);
}
11 changes: 5 additions & 6 deletions src/utilities/lexicographicSortSchema.js
Expand Up @@ -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')
Expand All @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions src/validation/rules/KnownTypeNamesRule.js
Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.js
Expand Up @@ -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(
Expand Down
8 changes: 4 additions & 4 deletions src/validation/rules/PossibleTypeExtensionsRule.js
Expand Up @@ -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(
Expand Down
30 changes: 13 additions & 17 deletions src/validation/rules/ProvidedRequiredArgumentsRule.js
Expand Up @@ -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(
Expand All @@ -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),
Expand Down Expand Up @@ -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,
),
);
Expand Down