Skip to content

Commit e56f9d5

Browse files
authored
Fix: OverlappingFieldsCanBeMerged on js keyword named fields (#864)
Replaces all places where `{}` is used as an empty string map with `Object.create(null)`.
1 parent d4222f9 commit e56f9d5

File tree

7 files changed

+44
-12
lines changed

7 files changed

+44
-12
lines changed

src/jsutils/keyMap.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,8 @@ export default function keyMap<T>(
3535
list: Array<T>,
3636
keyFn: (item: T) => string
3737
): {[key: string]: T} {
38-
return list.reduce((map, item) => ((map[keyFn(item)] = item), map), {});
38+
return list.reduce(
39+
(map, item) => ((map[keyFn(item)] = item), map),
40+
Object.create(null)
41+
);
3942
}

src/jsutils/keyValMap.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ export default function keyValMap<T, V>(
3232
): {[key: string]: V} {
3333
return list.reduce(
3434
(map, item) => ((map[keyFn(item)] = valFn(item)), map),
35-
{}
35+
Object.create(null)
3636
);
3737
}

src/type/definition.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ function defineInterfaces(
470470
'an Array.'
471471
);
472472

473-
const implementedTypeNames = {};
473+
const implementedTypeNames = Object.create(null);
474474
interfaces.forEach(iface => {
475475
invariant(
476476
iface instanceof GraphQLInterfaceType,
@@ -513,7 +513,7 @@ function defineFieldMap<TSource, TContext>(
513513
'function which returns such an object.'
514514
);
515515

516-
const resultFieldMap = {};
516+
const resultFieldMap = Object.create(null);
517517
fieldNames.forEach(fieldName => {
518518
assertValidName(fieldName);
519519
const fieldConfig = fieldMap[fieldName];
@@ -818,7 +818,7 @@ function defineTypes(
818818
'Must provide Array of types or a function which returns ' +
819819
`such an array for Union ${unionType.name}.`
820820
);
821-
const includedTypeNames = {};
821+
const includedTypeNames = Object.create(null);
822822
types.forEach(objType => {
823823
invariant(
824824
objType instanceof GraphQLObjectType,
@@ -1091,7 +1091,7 @@ export class GraphQLInputObjectType {
10911091
`${this.name} fields must be an object with field names as keys or a ` +
10921092
'function which returns such an object.'
10931093
);
1094-
const resultFieldMap = {};
1094+
const resultFieldMap = Object.create(null);
10951095
fieldNames.forEach(fieldName => {
10961096
assertValidName(fieldName);
10971097
const field = {

src/utilities/extendSchema.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ export function extendSchema(
123123
);
124124

125125
// Collect the type definitions and extensions found in the document.
126-
const typeDefinitionMap = {};
127-
const typeExtensionsMap = {};
126+
const typeDefinitionMap = Object.create(null);
127+
const typeExtensionsMap = Object.create(null);
128128

129129
// New directives and types are separate because a directives and types can
130130
// have the same name. For example, a type named "skip".
@@ -400,7 +400,7 @@ export function extendSchema(
400400
}
401401

402402
function extendFieldMap(type: GraphQLObjectType | GraphQLInterfaceType) {
403-
const newFieldMap = {};
403+
const newFieldMap = Object.create(null);
404404
const oldFieldMap = type.getFields();
405405
Object.keys(oldFieldMap).forEach(fieldName => {
406406
const field = oldFieldMap[fieldName];

src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,35 @@ describe('Validate: Overlapping fields can be merged', () => {
936936
);
937937
});
938938

939+
it('works for field names that are JS keywords', () => {
940+
const FooType = new GraphQLObjectType({
941+
name: 'Foo',
942+
fields: {
943+
constructor: {
944+
type: GraphQLString
945+
},
946+
}
947+
});
948+
949+
const schemaWithKeywords = new GraphQLSchema({
950+
query: new GraphQLObjectType({
951+
name: 'Query',
952+
fields: () => ({
953+
foo: { type: FooType },
954+
})
955+
}),
956+
});
957+
958+
expectPassesRuleWithSchema(
959+
schemaWithKeywords,
960+
OverlappingFieldsCanBeMerged,
961+
`{
962+
foo {
963+
constructor
964+
}
965+
}`
966+
);
967+
});
939968
});
940969

941970
});

src/validation/rules/OverlappingFieldsCanBeMerged.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -665,8 +665,8 @@ function getFieldsAndFragmentNames(
665665
): [ NodeAndDefCollection, Array<string> ] {
666666
let cached = cachedFieldsAndFragmentNames.get(selectionSet);
667667
if (!cached) {
668-
const nodeAndDefs = {};
669-
const fragmentNames = {};
668+
const nodeAndDefs = Object.create(null);
669+
const fragmentNames = Object.create(null);
670670
_collectFieldsAndFragmentNames(
671671
context,
672672
parentType,

src/validation/validate.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export class ValidationContext {
151151
frags[statement.name.value] = statement;
152152
}
153153
return frags;
154-
}, {});
154+
}, Object.create(null));
155155
}
156156
return fragments[name];
157157
}

0 commit comments

Comments
 (0)