Skip to content

Commit

Permalink
extensionASTNodes: always populate with empty array (#2996)
Browse files Browse the repository at this point in the history
  • Loading branch information
@IvanGoncharov
IvanGoncharov committed Mar 28, 2021
1 parent 491ddd5 commit a6a65b2
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 62 deletions.
12 changes: 6 additions & 6 deletions src/type/definition.d.ts
View file
Expand Up @@ -306,7 +306,7 @@ export class GraphQLScalarType {
parseLiteral: GraphQLScalarLiteralParser<any>;
extensions: Maybe<Readonly<GraphQLScalarTypeExtensions>>;
astNode: Maybe<ScalarTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<ScalarTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<ScalarTypeExtensionNode>;

constructor(config: Readonly<GraphQLScalarTypeConfig<any, any>>);

Expand Down Expand Up @@ -409,7 +409,7 @@ export class GraphQLObjectType<TSource = any, TContext = any> {
isTypeOf: Maybe<GraphQLIsTypeOfFn<TSource, TContext>>;
extensions: Maybe<Readonly<GraphQLObjectTypeExtensions<TSource, TContext>>>;
astNode: Maybe<ObjectTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<ObjectTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<ObjectTypeExtensionNode>;

constructor(config: Readonly<GraphQLObjectTypeConfig<TSource, TContext>>);

Expand Down Expand Up @@ -616,7 +616,7 @@ export class GraphQLInterfaceType {
resolveType: Maybe<GraphQLTypeResolver<any, any>>;
extensions: Maybe<Readonly<GraphQLInterfaceTypeExtensions>>;
astNode?: Maybe<InterfaceTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<InterfaceTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<InterfaceTypeExtensionNode>;

constructor(config: Readonly<GraphQLInterfaceTypeConfig<any, any>>);
getFields(): GraphQLFieldMap<any, any>;
Expand Down Expand Up @@ -692,7 +692,7 @@ export class GraphQLUnionType {
resolveType: Maybe<GraphQLTypeResolver<any, any>>;
extensions: Maybe<Readonly<GraphQLUnionTypeExtensions>>;
astNode: Maybe<UnionTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<UnionTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<UnionTypeExtensionNode>;

constructor(config: Readonly<GraphQLUnionTypeConfig<any, any>>);
getTypes(): Array<GraphQLObjectType>;
Expand Down Expand Up @@ -762,7 +762,7 @@ export class GraphQLEnumType {
description: Maybe<string>;
extensions: Maybe<Readonly<GraphQLEnumTypeExtensions>>;
astNode: Maybe<EnumTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<EnumTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<EnumTypeExtensionNode>;

constructor(config: Readonly<GraphQLEnumTypeConfig>);
getValues(): Array<GraphQLEnumValue>;
Expand Down Expand Up @@ -865,7 +865,7 @@ export class GraphQLInputObjectType {
description: Maybe<string>;
extensions: Maybe<Readonly<GraphQLInputObjectTypeExtensions>>;
astNode: Maybe<InputObjectTypeDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<InputObjectTypeExtensionNode>>;
extensionASTNodes: ReadonlyArray<InputObjectTypeExtensionNode>;

constructor(config: Readonly<GraphQLInputObjectTypeConfig>);
getFields(): GraphQLInputFieldMap;
Expand Down
40 changes: 18 additions & 22 deletions src/type/definition.js
View file
Expand Up @@ -522,10 +522,6 @@ function resolveObjMapThunk<T>(thunk: ThunkObjMap<T>): ObjMap<T> {
return typeof thunk === 'function' ? thunk() : thunk;
}
function undefineIfEmpty<T>(arr: ?$ReadOnlyArray<T>): ?$ReadOnlyArray<T> {
return arr && arr.length > 0 ? arr : undefined;
}
/**
* Scalar Type Definition
*
Expand Down Expand Up @@ -559,7 +555,7 @@ export class GraphQLScalarType {
parseLiteral: GraphQLScalarLiteralParser<mixed>;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?ScalarTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<ScalarTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<ScalarTypeExtensionNode>;

constructor(config: $ReadOnly<GraphQLScalarTypeConfig<mixed, mixed>>) {
const parseValue = config.parseValue ?? identityFunc;
Expand All @@ -573,7 +569,7 @@ export class GraphQLScalarType {
((node, variables) => parseValue(valueFromASTUntyped(node, variables)));
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

devAssert(typeof config.name === 'string', 'Must provide name.');

Expand Down Expand Up @@ -608,7 +604,7 @@ export class GraphQLScalarType {
parseLiteral: this.parseLiteral,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down Expand Up @@ -706,7 +702,7 @@ export class GraphQLObjectType {
isTypeOf: ?GraphQLIsTypeOfFn<any, any>;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?ObjectTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<ObjectTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<ObjectTypeExtensionNode>;

_fields: ThunkObjMap<GraphQLField<any, any>>;
_interfaces: ThunkArray<GraphQLInterfaceType>;
Expand All @@ -717,7 +713,7 @@ export class GraphQLObjectType {
this.isTypeOf = config.isTypeOf;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._fields = defineFieldMap.bind(undefined, config);
this._interfaces = defineInterfaces.bind(undefined, config);
Expand Down Expand Up @@ -752,7 +748,7 @@ export class GraphQLObjectType {
isTypeOf: this.isTypeOf,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes || [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down Expand Up @@ -1023,7 +1019,7 @@ export class GraphQLInterfaceType {
resolveType: ?GraphQLTypeResolver<any, any>;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?InterfaceTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<InterfaceTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<InterfaceTypeExtensionNode>;

_fields: ThunkObjMap<GraphQLField<any, any>>;
_interfaces: ThunkArray<GraphQLInterfaceType>;
Expand All @@ -1034,7 +1030,7 @@ export class GraphQLInterfaceType {
this.resolveType = config.resolveType;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._fields = defineFieldMap.bind(undefined, config);
this._interfaces = defineInterfaces.bind(undefined, config);
Expand Down Expand Up @@ -1069,7 +1065,7 @@ export class GraphQLInterfaceType {
resolveType: this.resolveType,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down Expand Up @@ -1140,7 +1136,7 @@ export class GraphQLUnionType {
resolveType: ?GraphQLTypeResolver<any, any>;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?UnionTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<UnionTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<UnionTypeExtensionNode>;

_types: ThunkArray<GraphQLObjectType>;

Expand All @@ -1150,7 +1146,7 @@ export class GraphQLUnionType {
this.resolveType = config.resolveType;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._types = defineTypes.bind(undefined, config);
devAssert(typeof config.name === 'string', 'Must provide name.');
Expand All @@ -1176,7 +1172,7 @@ export class GraphQLUnionType {
resolveType: this.resolveType,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down Expand Up @@ -1253,7 +1249,7 @@ export class GraphQLEnumType /* <T> */ {
description: ?string;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?EnumTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<EnumTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<EnumTypeExtensionNode>;

_values: Array<GraphQLEnumValue /* <T> */>;
_valueLookup: Map<any /* T */, GraphQLEnumValue>;
Expand All @@ -1264,7 +1260,7 @@ export class GraphQLEnumType /* <T> */ {
this.description = config.description;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._values = defineEnumValues(this.name, config.values);
this._valueLookup = new Map(
Expand Down Expand Up @@ -1354,7 +1350,7 @@ export class GraphQLEnumType /* <T> */ {
values,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down Expand Up @@ -1466,7 +1462,7 @@ export class GraphQLInputObjectType {
description: ?string;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?InputObjectTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<InputObjectTypeExtensionNode>;
extensionASTNodes: $ReadOnlyArray<InputObjectTypeExtensionNode>;

_fields: ThunkObjMap<GraphQLInputField>;

Expand All @@ -1475,7 +1471,7 @@ export class GraphQLInputObjectType {
this.description = config.description;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = undefineIfEmpty(config.extensionASTNodes);
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._fields = defineInputFieldMap.bind(undefined, config);
devAssert(typeof config.name === 'string', 'Must provide name.');
Expand Down Expand Up @@ -1503,7 +1499,7 @@ export class GraphQLInputObjectType {
fields,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/type/schema.d.ts
View file
Expand Up @@ -62,7 +62,7 @@ export class GraphQLSchema {
description: Maybe<string>;
extensions: Maybe<Readonly<GraphQLSchemaExtensions>>;
astNode: Maybe<SchemaDefinitionNode>;
extensionASTNodes: Maybe<ReadonlyArray<SchemaExtensionNode>>;
extensionASTNodes: ReadonlyArray<SchemaExtensionNode>;

constructor(config: Readonly<GraphQLSchemaConfig>);
getQueryType(): Maybe<GraphQLObjectType>;
Expand Down Expand Up @@ -136,6 +136,6 @@ export interface GraphQLSchemaNormalizedConfig extends GraphQLSchemaConfig {
types: Array<GraphQLNamedType>;
directives: Array<GraphQLDirective>;
extensions: Maybe<Readonly<GraphQLSchemaExtensions>>;
extensionASTNodes: Maybe<ReadonlyArray<SchemaExtensionNode>>;
extensionASTNodes: ReadonlyArray<SchemaExtensionNode>;
assumeValid: boolean;
}
6 changes: 3 additions & 3 deletions src/type/schema.js
View file
Expand Up @@ -121,7 +121,7 @@ export class GraphQLSchema {
description: ?string;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?SchemaDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<SchemaExtensionNode>;
extensionASTNodes: $ReadOnlyArray<SchemaExtensionNode>;

_queryType: ?GraphQLObjectType;
_mutationType: ?GraphQLObjectType;
Expand Down Expand Up @@ -157,7 +157,7 @@ export class GraphQLSchema {
this.description = config.description;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes;
this.extensionASTNodes = config.extensionASTNodes ?? [];

this._queryType = config.query;
this._mutationType = config.mutation;
Expand Down Expand Up @@ -337,7 +337,7 @@ export class GraphQLSchema {
directives: this.getDirectives().slice(),
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes ?? [],
extensionASTNodes: this.extensionASTNodes,
assumeValid: this.__validationErrors !== undefined,
};
}
Expand Down
34 changes: 20 additions & 14 deletions src/type/validate.js
View file
Expand Up @@ -261,10 +261,10 @@ function validateFields(

// Objects and Interfaces both must define one or more fields.
if (fields.length === 0) {
context.reportError(
`Type ${type.name} must define one or more fields.`,
[type.astNode].concat(type.extensionASTNodes),
);
context.reportError(`Type ${type.name} must define one or more fields.`, [
type.astNode,
...type.extensionASTNodes,
]);
}

for (const field of fields) {
Expand Down Expand Up @@ -364,7 +364,7 @@ function validateTypeImplementsInterface(
if (!typeField) {
context.reportError(
`Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`,
[ifaceField.astNode, type.astNode].concat(type.extensionASTNodes),
[ifaceField.astNode, type.astNode, ...type.extensionASTNodes],
);
continue;
}
Expand Down Expand Up @@ -464,7 +464,7 @@ function validateUnionMembers(
if (memberTypes.length === 0) {
context.reportError(
`Union type ${union.name} must define one or more member types.`,
[union.astNode].concat(union.extensionASTNodes),
[union.astNode, ...union.extensionASTNodes],
);
}

Expand Down Expand Up @@ -497,7 +497,7 @@ function validateEnumValues(
if (enumValues.length === 0) {
context.reportError(
`Enum type ${enumType.name} must define one or more values.`,
[enumType.astNode].concat(enumType.extensionASTNodes),
[enumType.astNode, ...enumType.extensionASTNodes],
);
}

Expand All @@ -524,7 +524,7 @@ function validateInputFields(
if (fields.length === 0) {
context.reportError(
`Input Object type ${inputObj.name} must define one or more fields.`,
[inputObj.astNode].concat(inputObj.extensionASTNodes),
[inputObj.astNode, ...inputObj.extensionASTNodes],
);
}

Expand Down Expand Up @@ -611,21 +611,27 @@ function getAllImplementsInterfaceNodes(
type: GraphQLObjectType | GraphQLInterfaceType,
iface: GraphQLInterfaceType,
): $ReadOnlyArray<NamedTypeNode> {
const { astNode, extensionASTNodes } = type;
const nodes =
astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes;

// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
return [type.astNode]
.concat(type.extensionASTNodes)
.flatMap((typeNode) => typeNode?.interfaces ?? [])
return nodes
.flatMap((typeNode) => typeNode.interfaces ?? [])
.filter((ifaceNode) => ifaceNode.name.value === iface.name);
}

function getUnionMemberTypeNodes(
union: GraphQLUnionType,
typeName: string,
): ?$ReadOnlyArray<NamedTypeNode> {
const { astNode, extensionASTNodes } = union;
const nodes =
astNode != null ? [astNode, ...extensionASTNodes] : extensionASTNodes;

// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2203')
return [union.astNode]
.concat(union.extensionASTNodes)
.flatMap((unionNode) => unionNode?.types ?? [])
return nodes
.flatMap((unionNode) => unionNode.types ?? [])
.filter((typeNode) => typeNode.name.value === typeName);
}

Expand Down
2 changes: 1 addition & 1 deletion src/utilities/__tests__/buildASTSchema-test.js
View file
Expand Up @@ -58,7 +58,7 @@ function printASTNode(obj: ?{ +astNode: ?ASTNode, ... }): string {
}

function printAllASTNodes(obj: GraphQLNamedType): string {
invariant(obj.astNode != null && obj.extensionASTNodes != null);
invariant(obj.astNode != null);
return print({
kind: Kind.DOCUMENT,
definitions: [obj.astNode, ...obj.extensionASTNodes],
Expand Down
20 changes: 6 additions & 14 deletions src/utilities/__tests__/extendSchema-test.js
View file
Expand Up @@ -38,8 +38,7 @@ import { extendSchema } from '../extendSchema';
import { buildSchema } from '../buildASTSchema';

function printExtensionNodes(obj: ?GraphQLNamedType | GraphQLSchema): string {
// istanbul ignore next (FIXME)
invariant(obj?.extensionASTNodes != null);
invariant(obj != null);
return print({
kind: Kind.DOCUMENT,
definitions: obj.extensionASTNodes,
Expand Down Expand Up @@ -442,18 +441,11 @@ describe('extendSchema', () => {
extendedTwiceSchema.getDirective('test'),
);

expect(testType).to.include({ extensionASTNodes: undefined });
expect(testEnum).to.include({ extensionASTNodes: undefined });
expect(testUnion).to.include({ extensionASTNodes: undefined });
expect(testInput).to.include({ extensionASTNodes: undefined });
expect(testInterface).to.include({ extensionASTNodes: undefined });

invariant(query.extensionASTNodes);
invariant(someScalar.extensionASTNodes);
invariant(someEnum.extensionASTNodes);
invariant(someUnion.extensionASTNodes);
invariant(someInput.extensionASTNodes);
invariant(someInterface.extensionASTNodes);
expect(testType.extensionASTNodes).to.deep.equal([]);
expect(testEnum.extensionASTNodes).to.deep.equal([]);
expect(testUnion.extensionASTNodes).to.deep.equal([]);
expect(testInput.extensionASTNodes).to.deep.equal([]);
expect(testInterface.extensionASTNodes).to.deep.equal([]);

expect([
testInput.astNode,
Expand Down

0 comments on commit a6a65b2

Please sign in to comment.