From 42f907271cf1a00010fb807b6ff9f647cea47600 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 22 Apr 2021 00:08:41 -0700 Subject: [PATCH] Address review feedback s/fieldName/memberName Adds wrapping object around return type of resolveSchemaCoordinate to ensure it's easy to use that result. --- src/index.d.ts | 2 +- src/index.js | 2 +- src/language/__tests__/parser-test.js | 10 +- src/language/ast.d.ts | 2 +- src/language/ast.js | 2 +- src/language/parser.js | 8 +- src/language/printer.js | 4 +- src/language/visitor.js | 2 +- src/type/element.d.ts | 16 -- src/type/element.js | 16 -- src/type/index.d.ts | 3 - src/type/index.js | 3 - .../__tests__/resolveSchemaCoordinate-test.js | 157 +++++++++++------- src/utilities/index.d.ts | 1 + src/utilities/index.js | 1 + src/utilities/resolveSchemaCoordinate.d.ts | 48 +++++- src/utilities/resolveSchemaCoordinate.js | 104 ++++++++++-- 17 files changed, 250 insertions(+), 131 deletions(-) delete mode 100644 src/type/element.d.ts delete mode 100644 src/type/element.js diff --git a/src/index.d.ts b/src/index.d.ts index 659bd51db5..949887353e 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -181,7 +181,6 @@ export { GraphQLScalarSerializer, GraphQLScalarValueParser, GraphQLScalarLiteralParser, - GraphQLSchemaElement, } from './type/index'; // Parse and operate on GraphQL language source files. @@ -462,4 +461,5 @@ export { BreakingChange, DangerousChange, TypedQueryDocumentNode, + GraphQLSchemaElement, } from './utilities/index'; diff --git a/src/index.js b/src/index.js index 712b6af67f..73b5e651ff 100644 --- a/src/index.js +++ b/src/index.js @@ -168,7 +168,6 @@ export type { GraphQLScalarSerializer, GraphQLScalarValueParser, GraphQLScalarLiteralParser, - GraphQLSchemaElement, } from './type/index'; // Parse and operate on GraphQL language source files. @@ -450,4 +449,5 @@ export type { BuildSchemaOptions, BreakingChange, DangerousChange, + GraphQLSchemaElement, } from './utilities/index'; diff --git a/src/language/__tests__/parser-test.js b/src/language/__tests__/parser-test.js index 81d1da42a1..bf993e4eaa 100644 --- a/src/language/__tests__/parser-test.js +++ b/src/language/__tests__/parser-test.js @@ -544,7 +544,7 @@ describe('Parser', () => { loc: { start: 0, end: 6 }, value: 'MyType', }, - fieldName: undefined, + memberName: undefined, argumentName: undefined, }); }); @@ -560,7 +560,7 @@ describe('Parser', () => { loc: { start: 0, end: 6 }, value: 'MyType', }, - fieldName: { + memberName: { kind: Kind.NAME, loc: { start: 7, end: 12 }, value: 'field', @@ -589,7 +589,7 @@ describe('Parser', () => { loc: { start: 0, end: 6 }, value: 'MyType', }, - fieldName: { + memberName: { kind: Kind.NAME, loc: { start: 7, end: 12 }, value: 'field', @@ -622,7 +622,7 @@ describe('Parser', () => { loc: { start: 1, end: 12 }, value: 'myDirective', }, - fieldName: undefined, + memberName: undefined, argumentName: undefined, }); }); @@ -638,7 +638,7 @@ describe('Parser', () => { loc: { start: 1, end: 12 }, value: 'myDirective', }, - fieldName: undefined, + memberName: undefined, argumentName: { kind: Kind.NAME, loc: { start: 13, end: 16 }, diff --git a/src/language/ast.d.ts b/src/language/ast.d.ts index eb029e7e05..e2b544fcc3 100644 --- a/src/language/ast.d.ts +++ b/src/language/ast.d.ts @@ -609,6 +609,6 @@ export interface SchemaCoordinateNode { readonly loc?: Location; readonly isDirective: boolean; readonly name: NameNode; - readonly fieldName?: NameNode; + readonly memberName?: NameNode; readonly argumentName?: NameNode; } diff --git a/src/language/ast.js b/src/language/ast.js index 14f29aba0f..f64f5365fc 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -635,6 +635,6 @@ export type SchemaCoordinateNode = {| +loc?: Location, +isDirective: boolean, +name: NameNode, - +fieldName?: NameNode, + +memberName?: NameNode, +argumentName?: NameNode, |}; diff --git a/src/language/parser.js b/src/language/parser.js index c20da969dc..56e07ed60c 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -1371,13 +1371,13 @@ export class Parser { const start = this._lexer.token; const isDirective = this.expectOptionalToken(TokenKind.AT); const name = this.parseName(); - let fieldName; + let memberName; if (!isDirective && this.expectOptionalToken(TokenKind.DOT)) { - fieldName = this.parseName(); + memberName = this.parseName(); } let argumentName; if ( - (isDirective || fieldName) && + (isDirective || memberName) && this.expectOptionalToken(TokenKind.PAREN_L) ) { argumentName = this.parseName(); @@ -1388,7 +1388,7 @@ export class Parser { kind: Kind.SCHEMA_COORDINATE, isDirective, name, - fieldName, + memberName, argumentName, loc: this.loc(start), }; diff --git a/src/language/printer.js b/src/language/printer.js index 5ec76ca763..d92fb065e0 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -305,11 +305,11 @@ const printDocASTReducer: any = { // Schema Coordinate SchemaCoordinate: { - leave: ({ isDirective, name, fieldName, argumentName }) => + leave: ({ isDirective, name, memberName, argumentName }) => join([ isDirective && '@', name, - wrap('.', fieldName), + wrap('.', memberName), wrap('(', argumentName, ':)'), ]), }, diff --git a/src/language/visitor.js b/src/language/visitor.js index 313a637f43..1f8ff56bda 100644 --- a/src/language/visitor.js +++ b/src/language/visitor.js @@ -123,7 +123,7 @@ const QueryDocumentKeys = { EnumTypeExtension: ['name', 'directives', 'values'], InputObjectTypeExtension: ['name', 'directives', 'fields'], - SchemaCoordinate: ['name', 'fieldName', 'argumentName'], + SchemaCoordinate: ['name', 'memberName', 'argumentName'], }; export const BREAK: { ... } = Object.freeze({}); diff --git a/src/type/element.d.ts b/src/type/element.d.ts deleted file mode 100644 index 1fad2b741a..0000000000 --- a/src/type/element.d.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { - GraphQLNamedType, - GraphQLField, - GraphQLInputField, - GraphQLEnumValue, - GraphQLArgument, -} from './definition'; -import { GraphQLDirective } from './directives'; - -export type GraphQLSchemaElement = - | GraphQLNamedType - | GraphQLField - | GraphQLInputField - | GraphQLEnumValue - | GraphQLArgument - | GraphQLDirective; diff --git a/src/type/element.js b/src/type/element.js deleted file mode 100644 index d5a2b4a5ea..0000000000 --- a/src/type/element.js +++ /dev/null @@ -1,16 +0,0 @@ -import type { - GraphQLNamedType, - GraphQLField, - GraphQLInputField, - GraphQLEnumValue, - GraphQLArgument, -} from './definition'; -import type { GraphQLDirective } from './directives'; - -export type GraphQLSchemaElement = - | GraphQLNamedType - | GraphQLField - | GraphQLInputField - | GraphQLEnumValue - | GraphQLArgument - | GraphQLDirective; diff --git a/src/type/index.d.ts b/src/type/index.d.ts index cf26feee90..f226df5651 100644 --- a/src/type/index.d.ts +++ b/src/type/index.d.ts @@ -167,6 +167,3 @@ export { } from './introspection'; export { validateSchema, assertValidSchema } from './validate'; - -// Schema Element type. -export { GraphQLSchemaElement } from './element'; diff --git a/src/type/index.js b/src/type/index.js index 68aac92d24..6bcef9daed 100644 --- a/src/type/index.js +++ b/src/type/index.js @@ -161,6 +161,3 @@ export type { // Validate GraphQL schema. export { validateSchema, assertValidSchema } from './validate'; - -// Schema Element type. -export type { GraphQLSchemaElement } from './element'; diff --git a/src/utilities/__tests__/resolveSchemaCoordinate-test.js b/src/utilities/__tests__/resolveSchemaCoordinate-test.js index 71c625c01c..735ad5f9ec 100644 --- a/src/utilities/__tests__/resolveSchemaCoordinate-test.js +++ b/src/utilities/__tests__/resolveSchemaCoordinate-test.js @@ -31,116 +31,147 @@ describe('resolveSchemaCoordinate', () => { `); it('resolves a Named Type', () => { - const expected = schema.getType('Business'); - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'Business')).to.equal(expected); + expect(resolveSchemaCoordinate(schema, 'Business')).to.deep.equal({ + kind: 'NamedType', + type: schema.getType('Business'), + }); - expect(resolveSchemaCoordinate(schema, 'String')).to.equal( - schema.getType('String'), - ); + expect(resolveSchemaCoordinate(schema, 'String')).to.deep.equal({ + kind: 'NamedType', + type: schema.getType('String'), + }); - expect(resolveSchemaCoordinate(schema, 'private')).to.equal(undefined); + expect(resolveSchemaCoordinate(schema, 'private')).to.deep.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'Unknown')).to.equal(undefined); + expect(resolveSchemaCoordinate(schema, 'Unknown')).to.deep.equal(undefined); }); it('resolves a Type Field', () => { - const expected = schema.getType('Business').getFields().name; - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'Business.name')).to.equal(expected); - - expect(resolveSchemaCoordinate(schema, 'Business.unknown')).to.equal( + const type = schema.getType('Business'); + const field = type.getFields().name; + expect(resolveSchemaCoordinate(schema, 'Business.name')).to.deep.equal({ + kind: 'Field', + type, + field, + }); + + expect(resolveSchemaCoordinate(schema, 'Business.unknown')).to.deep.equal( undefined, ); - expect(resolveSchemaCoordinate(schema, 'Unknown.field')).to.equal( + expect(resolveSchemaCoordinate(schema, 'Unknown.field')).to.deep.equal( undefined, ); - expect(resolveSchemaCoordinate(schema, 'String.field')).to.equal(undefined); + expect(resolveSchemaCoordinate(schema, 'String.field')).to.deep.equal( + undefined, + ); }); it('does not resolve meta-fields', () => { - expect(resolveSchemaCoordinate(schema, 'Business.__typename')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'Business.__typename'), + ).to.deep.equal(undefined); }); it('resolves a Input Field', () => { - const expected = schema.getType('SearchCriteria').getFields().filter; - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'SearchCriteria.filter')).to.equal( - expected, - ); + const type = schema.getType('SearchCriteria'); + const inputField = type.getFields().filter; + expect( + resolveSchemaCoordinate(schema, 'SearchCriteria.filter'), + ).to.deep.equal({ + kind: 'InputField', + type, + inputField, + }); - expect(resolveSchemaCoordinate(schema, 'SearchCriteria.unknown')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'SearchCriteria.unknown'), + ).to.deep.equal(undefined); }); it('resolves a Enum Value', () => { - const expected = schema.getType('SearchFilter').getValue('OPEN_NOW'); - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'SearchFilter.OPEN_NOW')).to.equal( - expected, - ); + const type = schema.getType('SearchFilter'); + const enumValue = type.getValue('OPEN_NOW'); + expect( + resolveSchemaCoordinate(schema, 'SearchFilter.OPEN_NOW'), + ).to.deep.equal({ + kind: 'EnumValue', + type, + enumValue, + }); - expect(resolveSchemaCoordinate(schema, 'SearchFilter.UNKNOWN')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'SearchFilter.UNKNOWN'), + ).to.deep.equal(undefined); }); it('resolves a Field Argument', () => { - const expected = schema - .getType('Query') - .getFields() - .searchBusiness.args.find((arg) => arg.name === 'criteria'); - expect(expected).not.to.equal(undefined); + const type = schema.getType('Query'); + const field = type.getFields().searchBusiness; + const fieldArgument = field.args.find((arg) => arg.name === 'criteria'); expect( resolveSchemaCoordinate(schema, 'Query.searchBusiness(criteria:)'), - ).to.equal(expected); + ).to.deep.equal({ + kind: 'FieldArgument', + type, + field, + fieldArgument, + }); - expect(resolveSchemaCoordinate(schema, 'Business.name(unknown:)')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'Business.name(unknown:)'), + ).to.deep.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'Unknown.field(arg:)')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'Unknown.field(arg:)'), + ).to.deep.equal(undefined); - expect(resolveSchemaCoordinate(schema, 'Business.unknown(arg:)')).to.equal( - undefined, - ); + expect( + resolveSchemaCoordinate(schema, 'Business.unknown(arg:)'), + ).to.deep.equal(undefined); expect( resolveSchemaCoordinate(schema, 'SearchCriteria.name(arg:)'), - ).to.equal(undefined); + ).to.deep.equal(undefined); }); it('resolves a Directive', () => { - const expected = schema.getDirective('private'); - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, '@private')).to.equal(expected); + expect(resolveSchemaCoordinate(schema, '@private')).to.deep.equal({ + kind: 'Directive', + directive: schema.getDirective('private'), + }); - expect(resolveSchemaCoordinate(schema, '@unknown')).to.equal(undefined); + expect(resolveSchemaCoordinate(schema, '@deprecated')).to.deep.equal({ + kind: 'Directive', + directive: schema.getDirective('deprecated'), + }); - expect(resolveSchemaCoordinate(schema, '@Business')).to.equal(undefined); + expect(resolveSchemaCoordinate(schema, '@unknown')).to.deep.equal( + undefined, + ); + + expect(resolveSchemaCoordinate(schema, '@Business')).to.deep.equal( + undefined, + ); }); it('resolves a Directive Argument', () => { - const expected = schema - .getDirective('private') - .args.find((arg) => arg.name === 'scope'); - expect(expected).not.to.equal(undefined); - expect(resolveSchemaCoordinate(schema, '@private(scope:)')).to.equal( - expected, + const directive = schema.getDirective('private'); + const directiveArgument = directive.args.find( + (arg) => arg.name === 'scope', ); + expect(resolveSchemaCoordinate(schema, '@private(scope:)')).to.deep.equal({ + kind: 'DirectiveArgument', + directive, + directiveArgument, + }); - expect(resolveSchemaCoordinate(schema, '@private(unknown:)')).to.equal( + expect(resolveSchemaCoordinate(schema, '@private(unknown:)')).to.deep.equal( undefined, ); - expect(resolveSchemaCoordinate(schema, '@unknown(arg:)')).to.equal( + expect(resolveSchemaCoordinate(schema, '@unknown(arg:)')).to.deep.equal( undefined, ); }); diff --git a/src/utilities/index.d.ts b/src/utilities/index.d.ts index c90eed997c..e407e05324 100644 --- a/src/utilities/index.d.ts +++ b/src/utilities/index.d.ts @@ -112,6 +112,7 @@ export { TypedQueryDocumentNode } from './typedQueryDocumentNode'; // Schema coordinates export { + GraphQLSchemaElement, resolveSchemaCoordinate, resolveASTSchemaCoordinate, } from './resolveSchemaCoordinate'; diff --git a/src/utilities/index.js b/src/utilities/index.js index 2e9da2d63e..b76978aa93 100644 --- a/src/utilities/index.js +++ b/src/utilities/index.js @@ -106,6 +106,7 @@ export type { BreakingChange, DangerousChange } from './findBreakingChanges'; // Schema coordinates export { + GraphQLSchemaElement, resolveSchemaCoordinate, resolveASTSchemaCoordinate, } from './resolveSchemaCoordinate'; diff --git a/src/utilities/resolveSchemaCoordinate.d.ts b/src/utilities/resolveSchemaCoordinate.d.ts index 68567c5616..02d5703e05 100644 --- a/src/utilities/resolveSchemaCoordinate.d.ts +++ b/src/utilities/resolveSchemaCoordinate.d.ts @@ -1,8 +1,54 @@ import { GraphQLSchema } from '../type/schema'; -import { GraphQLSchemaElement } from '../type/element'; +import { + GraphQLNamedType, + GraphQLField, + GraphQLInputField, + GraphQLEnumValue, + GraphQLArgument, +} from '../type/definition'; +import { GraphQLDirective } from '../type/directives'; import { SchemaCoordinateNode } from '../language/ast'; import { Source } from '../language/source'; +/** + * A schema element may be one of the following kinds: + */ +export type GraphQLSchemaElement = + | Readonly<{ + kind: 'NamedType'; + type: GraphQLNamedType; + }> + | Readonly<{ + kind: 'Field'; + type: GraphQLNamedType; + field: GraphQLField; + }> + | Readonly<{ + kind: 'InputField'; + type: GraphQLNamedType; + inputField: GraphQLInputField; + }> + | Readonly<{ + kind: 'EnumValue'; + type: GraphQLNamedType; + enumValue: GraphQLEnumValue; + }> + | Readonly<{ + kind: 'FieldArgument'; + type: GraphQLNamedType; + field: GraphQLField; + fieldArgument: GraphQLArgument; + }> + | Readonly<{ + kind: 'Directive'; + directive: GraphQLDirective; + }> + | Readonly<{ + kind: 'DirectiveArgument'; + directive: GraphQLDirective; + directiveArgument: GraphQLArgument; + }>; + /** * A schema coordinate is resolved in the context of a GraphQL schema to * uniquely identifies a schema element. It returns undefined if the schema diff --git a/src/utilities/resolveSchemaCoordinate.js b/src/utilities/resolveSchemaCoordinate.js index bb268909ff..17e999e091 100644 --- a/src/utilities/resolveSchemaCoordinate.js +++ b/src/utilities/resolveSchemaCoordinate.js @@ -1,5 +1,4 @@ import type { GraphQLSchema } from '../type/schema'; -import type { GraphQLSchemaElement } from '../type/element'; import type { SchemaCoordinateNode } from '../language/ast'; import type { Source } from '../language/source'; import { @@ -9,6 +8,53 @@ import { isInputObjectType, } from '../type/definition'; import { parseSchemaCoordinate } from '../language/parser'; +import type { + GraphQLNamedType, + GraphQLField, + GraphQLInputField, + GraphQLEnumValue, + GraphQLArgument, +} from '../type/definition'; +import type { GraphQLDirective } from '../type/directives'; + +/** + * A schema element may be one of the following kinds: + */ +export type GraphQLSchemaElement = + | {| + kind: 'NamedType', + type: GraphQLNamedType, + |} + | {| + kind: 'Field', + type: GraphQLNamedType, + field: GraphQLField, + |} + | {| + kind: 'InputField', + type: GraphQLNamedType, + inputField: GraphQLInputField, + |} + | {| + kind: 'EnumValue', + type: GraphQLNamedType, + enumValue: GraphQLEnumValue, + |} + | {| + kind: 'FieldArgument', + type: GraphQLNamedType, + field: GraphQLField, + fieldArgument: GraphQLArgument, + |} + | {| + kind: 'Directive', + directive: GraphQLDirective, + |} + | {| + kind: 'DirectiveArgument', + directive: GraphQLDirective, + directiveArgument: GraphQLArgument, + |}; /** * A schema coordinate is resolved in the context of a GraphQL schema to @@ -34,7 +80,7 @@ export function resolveASTSchemaCoordinate( schema: GraphQLSchema, schemaCoordinate: SchemaCoordinateNode, ): GraphQLSchemaElement | void { - const { isDirective, name, fieldName, argumentName } = schemaCoordinate; + const { isDirective, name, memberName, argumentName } = schemaCoordinate; if (isDirective) { // SchemaCoordinate : // - @ Name @@ -45,7 +91,10 @@ export function resolveASTSchemaCoordinate( if (!argumentName) { // SchemaCoordinate : @ Name // Return the directive in the {schema} named {directiveName}. - return directive || undefined; + if (!directive) { + return; + } + return { kind: 'Directive', directive }; } // SchemaCoordinate : @ Name ( Name : ) @@ -53,7 +102,15 @@ export function resolveASTSchemaCoordinate( if (!directive) { return; } - return directive.args.find((arg) => arg.name === argumentName.value); + // Let {directiveArgumentName} be the value of the second {Name}. + // Return the argument of {directive} named {directiveArgumentName}. + const directiveArgument = directive.args.find( + (arg) => arg.name === argumentName.value, + ); + if (!directiveArgument) { + return; + } + return { kind: 'DirectiveArgument', directive, directiveArgument }; } // SchemaCoordinate : @@ -63,10 +120,13 @@ export function resolveASTSchemaCoordinate( // Let {typeName} be the value of the first {Name}. // Let {type} be the type in the {schema} named {typeName}. const type = schema.getType(name.value); - if (!fieldName) { + if (!memberName) { // SchemaCoordinate : Name // Return the type in the {schema} named {typeName}. - return type || undefined; + if (!type) { + return; + } + return { kind: 'NamedType', type }; } if (!argumentName) { @@ -75,13 +135,21 @@ export function resolveASTSchemaCoordinate( if (isEnumType(type)) { // Let {enumValueName} be the value of the second {Name}. // Return the enum value of {type} named {enumValueName}. - return type.getValue(fieldName.value) || undefined; + const enumValue = type.getValue(memberName.value); + if (!enumValue) { + return; + } + return { kind: 'EnumValue', type, enumValue }; } // Otherwise if {type} is an Input Object type: if (isInputObjectType(type)) { // Let {inputFieldName} be the value of the second {Name}. // Return the input field of {type} named {inputFieldName}. - return type.getFields()[fieldName.value]; + const inputField = type.getFields()[memberName.value]; + if (!inputField) { + return; + } + return { kind: 'InputField', type, inputField }; } // Otherwise: // Assert {type} must be an Object or Interface type. @@ -90,7 +158,11 @@ export function resolveASTSchemaCoordinate( } // Let {fieldName} be the value of the second {Name}. // Return the field of {type} named {fieldName}. - return type.getFields()[fieldName.value]; + const field = type.getFields()[memberName.value]; + if (!field) { + return; + } + return { kind: 'Field', type, field }; } // SchemaCoordinate : Name . Name ( Name : ) @@ -100,12 +172,18 @@ export function resolveASTSchemaCoordinate( } // Let {fieldName} be the value of the second {Name}. // Let {field} be the field of {type} named {fieldName}. - const field = type.getFields()[fieldName.value]; + const field = type.getFields()[memberName.value]; // Assert {field} must exist. if (!field) { return; } - // Let {argumentName} be the value of the third {Name}. - // Return the argument of {field} named {argumentName}. - return field.args.find((arg) => arg.name === argumentName.value); + // Let {fieldArgumentName} be the value of the third {Name}. + // Return the argument of {field} named {fieldArgumentName}. + const fieldArgument = field.args.find( + (arg) => arg.name === argumentName.value, + ); + if (!fieldArgument) { + return; + } + return { kind: 'FieldArgument', type, field, fieldArgument }; }