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.

Sign up for GitHub

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

Jump to bottom

extensionASTNodes: always populate with empty array #2996

Merged
merged 1 commit into from Mar 28, 2021
Merged

extensionASTNodes: always populate with empty array #2996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions src/type/definition.d.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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