Skip to content

Commit 433774d

Browse files
IvanGoncharovleebyron
authored andcommitted
Fix execute deoptimisation caused by GraphQLEnumType (#1288)
1 parent bf3d27c commit 433774d

File tree

2 files changed

+24
-44
lines changed

2 files changed

+24
-44
lines changed

src/type/__tests__/definition-test.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,47 +1048,47 @@ describe('Type System: Enum types must be well defined', () => {
10481048
});
10491049

10501050
it('rejects an Enum type with incorrectly typed values', () => {
1051-
const enumType = new GraphQLEnumType({
1051+
const config = {
10521052
name: 'SomeEnum',
10531053
values: [{ FOO: 10 }],
1054-
});
1055-
expect(() => enumType.getValue()).to.throw(
1054+
};
1055+
expect(() => new GraphQLEnumType(config)).to.throw(
10561056
'SomeEnum values must be an object with value names as keys.',
10571057
);
10581058
});
10591059

10601060
it('rejects an Enum type with missing value definition', () => {
1061-
const enumType = new GraphQLEnumType({
1061+
const config = {
10621062
name: 'SomeEnum',
10631063
values: { FOO: null },
1064-
});
1065-
expect(() => enumType.getValues()).to.throw(
1064+
};
1065+
expect(() => new GraphQLEnumType(config)).to.throw(
10661066
'SomeEnum.FOO must refer to an object with a "value" key representing ' +
10671067
'an internal value but got: null.',
10681068
);
10691069
});
10701070

10711071
it('rejects an Enum type with incorrectly typed value definition', () => {
1072-
const enumType = new GraphQLEnumType({
1072+
const config = {
10731073
name: 'SomeEnum',
10741074
values: { FOO: 10 },
1075-
});
1076-
expect(() => enumType.getValues()).to.throw(
1075+
};
1076+
expect(() => new GraphQLEnumType(config)).to.throw(
10771077
'SomeEnum.FOO must refer to an object with a "value" key representing ' +
10781078
'an internal value but got: 10.',
10791079
);
10801080
});
10811081

10821082
it('does not allow isDeprecated without deprecationReason on enum', () => {
1083-
const enumType = new GraphQLEnumType({
1083+
const config = {
10841084
name: 'SomeEnum',
10851085
values: {
10861086
FOO: {
10871087
isDeprecated: true,
10881088
},
10891089
},
1090-
});
1091-
expect(() => enumType.getValues()).to.throw(
1090+
};
1091+
expect(() => new GraphQLEnumType(config)).to.throw(
10921092
'SomeEnum.FOO should provide "deprecationReason" instead ' +
10931093
'of "isDeprecated".',
10941094
);

src/type/definition.js

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import instanceOf from '../jsutils/instanceOf';
1111
import invariant from '../jsutils/invariant';
1212
import isInvalid from '../jsutils/isInvalid';
13+
import keyMap from '../jsutils/keyMap';
1314
import type { ObjMap } from '../jsutils/ObjMap';
1415
import { Kind } from '../language/kinds';
1516
import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped';
@@ -1071,7 +1072,6 @@ export class GraphQLEnumType /* <T> */ {
10711072
description: ?string;
10721073
astNode: ?EnumTypeDefinitionNode;
10731074

1074-
_enumConfig: GraphQLEnumTypeConfig /* <T> */;
10751075
_values: Array<GraphQLEnumValue /* <T> */>;
10761076
_valueLookup: Map<any /* T */, GraphQLEnumValue>;
10771077
_nameLookup: ObjMap<GraphQLEnumValue>;
@@ -1080,31 +1080,33 @@ export class GraphQLEnumType /* <T> */ {
10801080
this.name = config.name;
10811081
this.description = config.description;
10821082
this.astNode = config.astNode;
1083-
this._enumConfig = config;
1083+
this._values = defineEnumValues(this, config.values);
1084+
this._valueLookup = new Map(
1085+
this._values.map(enumValue => [enumValue.value, enumValue]),
1086+
);
1087+
this._nameLookup = keyMap(this._values, value => value.name);
1088+
10841089
invariant(typeof config.name === 'string', 'Must provide name.');
10851090
}
10861091

10871092
getValues(): Array<GraphQLEnumValue /* <T> */> {
1088-
return (
1089-
this._values ||
1090-
(this._values = defineEnumValues(this, this._enumConfig.values))
1091-
);
1093+
return this._values;
10921094
}
10931095

10941096
getValue(name: string): ?GraphQLEnumValue {
1095-
return this._getNameLookup()[name];
1097+
return this._nameLookup[name];
10961098
}
10971099

10981100
serialize(value: any /* T */): ?string {
1099-
const enumValue = this._getValueLookup().get(value);
1101+
const enumValue = this._valueLookup.get(value);
11001102
if (enumValue) {
11011103
return enumValue.name;
11021104
}
11031105
}
11041106

11051107
parseValue(value: mixed): ?any /* T */ {
11061108
if (typeof value === 'string') {
1107-
const enumValue = this._getNameLookup()[value];
1109+
const enumValue = this.getValue(value);
11081110
if (enumValue) {
11091111
return enumValue.value;
11101112
}
@@ -1114,35 +1116,13 @@ export class GraphQLEnumType /* <T> */ {
11141116
parseLiteral(valueNode: ValueNode, _variables: ?ObjMap<mixed>): ?any /* T */ {
11151117
// Note: variables will be resolved to a value before calling this function.
11161118
if (valueNode.kind === Kind.ENUM) {
1117-
const enumValue = this._getNameLookup()[valueNode.value];
1119+
const enumValue = this.getValue(valueNode.value);
11181120
if (enumValue) {
11191121
return enumValue.value;
11201122
}
11211123
}
11221124
}
11231125

1124-
_getValueLookup(): Map<any /* T */, GraphQLEnumValue> {
1125-
if (!this._valueLookup) {
1126-
const lookup = new Map();
1127-
this.getValues().forEach(value => {
1128-
lookup.set(value.value, value);
1129-
});
1130-
this._valueLookup = lookup;
1131-
}
1132-
return this._valueLookup;
1133-
}
1134-
1135-
_getNameLookup(): ObjMap<GraphQLEnumValue> {
1136-
if (!this._nameLookup) {
1137-
const lookup = Object.create(null);
1138-
this.getValues().forEach(value => {
1139-
lookup[value.name] = value;
1140-
});
1141-
this._nameLookup = lookup;
1142-
}
1143-
return this._nameLookup;
1144-
}
1145-
11461126
toString(): string {
11471127
return this.name;
11481128
}

0 commit comments

Comments
 (0)