From 941a13564978f60671098377f9c01334f757e690 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 14:08:40 -0800 Subject: [PATCH 1/8] Propagate fields from interfaces (make it work) --- ...atExistsOnConcreteType.invalid.ts.expected | 12 +- ...nImplementingInterface.invalid.ts.expected | 12 +- ...efinesInterfaceFieldWithAlternateMethod.ts | 18 ++ ...erfaceFieldWithAlternateMethod.ts.expected | 68 +++++++ ...RefinesInterfaceFieldWithNewDescription.ts | 18 ++ ...terfaceFieldWithNewDescription.ts.expected | 69 +++++++ ...eteTypeRefinesInterfaceFieldWithSubtype.ts | 13 ++ ...finesInterfaceFieldWithSubtype.ts.expected | 60 +++++++ .../interfaceFieldsInheritedByConcreteType.ts | 11 ++ ...eFieldsInheritedByConcreteType.ts.expected | 58 ++++++ src/transforms/mergeExtensions.ts | 170 +++++++++++++----- 11 files changed, 454 insertions(+), 55 deletions(-) create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts.expected create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts.expected create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts.expected create mode 100644 src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts create mode 100644 src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts.expected diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected index ec598487..9c3ecc21 100644 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected @@ -29,12 +29,12 @@ class User implements IPerson { ----------------- OUTPUT ----------------- -src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:21:3 - error: Field "User.greeting" can only be defined once. +src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:9:17 - error: Field "User.greeting" can only be defined once. -21 greeting(): string { - ~~~~~~~~ +9 export function greeting(person: IPerson): string { + ~~~~~~~~ - src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:9:17 - 9 export function greeting(person: IPerson): string { - ~~~~~~~~ + src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:21:3 + 21 greeting(): string { + ~~~~~~~~ Related location diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected index 21db8a89..fc811d88 100644 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected @@ -27,12 +27,12 @@ interface User extends IPerson { ----------------- OUTPUT ----------------- -src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:21:3 - error: Field "User.greeting" can only be defined once. +src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:9:17 - error: Field "User.greeting" can only be defined once. -21 greeting(): string; - ~~~~~~~~ +9 export function greeting(person: IPerson): string { + ~~~~~~~~ - src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:9:17 - 9 export function greeting(person: IPerson): string { - ~~~~~~~~ + src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:21:3 + 21 greeting(): string; + ~~~~~~~~ Related location diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts new file mode 100644 index 00000000..ca5dd94e --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts @@ -0,0 +1,18 @@ +// { "nullableByDefault": false } +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + name: string; + /** + * @gqlField name + */ + someOtherMethod(): string { + return this.name + "!"; + } +} diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts.expected b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts.expected new file mode 100644 index 00000000..77516f56 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithAlternateMethod.ts.expected @@ -0,0 +1,68 @@ +----------------- +INPUT +----------------- +// { "nullableByDefault": false } +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + name: string; + /** + * @gqlField name + */ + someOtherMethod(): string { + return this.name + "!"; + } +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IThing { + name: String @metadata +} + +type Doohickey implements IThing { + name: String! @metadata(argCount: 0, name: "someOtherMethod") +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType, GraphQLNonNull } from "graphql"; +export function getSchema(): GraphQLSchema { + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + name: "name", + type: new GraphQLNonNull(GraphQLString), + resolve(source, args, context, info) { + return source.someOtherMethod(source, args, context, info); + } + } + }; + }, + interfaces() { + return [IThingType]; + } + }); + return new GraphQLSchema({ + types: [IThingType, DoohickeyType] + }); +} diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts new file mode 100644 index 00000000..09381e41 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts @@ -0,0 +1,18 @@ +/** @gqlInterface */ +interface IThing { + /** + * This description is on the interface type. + * @gqlField + */ + name: string; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + /** + * This description is on the concrete type. + * @gqlField + */ + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts.expected b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts.expected new file mode 100644 index 00000000..e49550b8 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithNewDescription.ts.expected @@ -0,0 +1,69 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface IThing { + /** + * This description is on the interface type. + * @gqlField + */ + name: string; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + /** + * This description is on the concrete type. + * @gqlField + */ + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IThing { + """This description is on the interface type.""" + name: String @metadata +} + +type Doohickey implements IThing { + """This description is on the concrete type.""" + name: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + name: { + description: "This description is on the interface type.", + name: "name", + type: GraphQLString + } + }; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + description: "This description is on the concrete type.", + name: "name", + type: GraphQLString + } + }; + }, + interfaces() { + return [IThingType]; + } + }); + return new GraphQLSchema({ + types: [IThingType, DoohickeyType] + }); +} diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts new file mode 100644 index 00000000..b0aa3ea3 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts @@ -0,0 +1,13 @@ +// { "nullableByDefault": false } +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + /** @gqlField */ + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts.expected b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts.expected new file mode 100644 index 00000000..6d3cb191 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeRefinesInterfaceFieldWithSubtype.ts.expected @@ -0,0 +1,60 @@ +----------------- +INPUT +----------------- +// { "nullableByDefault": false } +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing { + __typename: "Doohickey"; + /** @gqlField */ + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IThing { + name: String @metadata +} + +type Doohickey implements IThing { + name: String! @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType, GraphQLNonNull } from "graphql"; +export function getSchema(): GraphQLSchema { + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + name: "name", + type: new GraphQLNonNull(GraphQLString) + } + }; + }, + interfaces() { + return [IThingType]; + } + }); + return new GraphQLSchema({ + types: [IThingType, DoohickeyType] + }); +} diff --git a/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts b/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts new file mode 100644 index 00000000..42637b77 --- /dev/null +++ b/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts @@ -0,0 +1,11 @@ +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string; +} + +/** @gqlType */ +class Doohickey implements IThing { + __typename: "Doohickey"; + name: string; +} diff --git a/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts.expected b/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts.expected new file mode 100644 index 00000000..3752abaa --- /dev/null +++ b/src/tests/fixtures/interfaces/interfaceFieldsInheritedByConcreteType.ts.expected @@ -0,0 +1,58 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface IThing { + /** @gqlField */ + name: string; +} + +/** @gqlType */ +class Doohickey implements IThing { + __typename: "Doohickey"; + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IThing { + name: String @metadata +} + +type Doohickey implements IThing { + name: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + }, + interfaces() { + return [IThingType]; + } + }); + return new GraphQLSchema({ + types: [IThingType, DoohickeyType] + }); +} diff --git a/src/transforms/mergeExtensions.ts b/src/transforms/mergeExtensions.ts index 48a4320f..47f2d055 100644 --- a/src/transforms/mergeExtensions.ts +++ b/src/transforms/mergeExtensions.ts @@ -1,58 +1,142 @@ import { DocumentNode, FieldDefinitionNode, visit } from "graphql"; -import { extend } from "../utils/helpers"; +import { DefaultMap, extend } from "../utils/helpers"; /** * Takes every example of `extend type Foo` and `extend interface Foo` and * merges them into the original type/interface definition. */ export function mergeExtensions(doc: DocumentNode): DocumentNode { - const fields = new MultiMap(); + const merger = new ExtensionMerger(); + return merger.mergeExtensions(doc); +} - // Collect all the fields from the extensions and trim them from the AST. - const sansExtensions = visit(doc, { - ObjectTypeExtension(t) { - if (t.directives != null || t.interfaces != null) { - throw new Error("Unexpected directives or interfaces on Extension"); - } - fields.extend(t.name.value, t.fields); - return null; - }, - InterfaceTypeExtension(t) { - if (t.directives != null || t.interfaces != null) { - throw new Error("Unexpected directives or interfaces on Extension"); - } - fields.extend(t.name.value, t.fields); - return null; - }, - // Grats does not create these extension types - ScalarTypeExtension(_) { - throw new Error("Unexpected ScalarTypeExtension"); - }, - EnumTypeExtension(_) { - throw new Error("Unexpected EnumTypeExtension"); - }, - SchemaExtension(_) { - throw new Error("Unexpected SchemaExtension"); - }, - }); +class ExtensionMerger { + _fields: MultiMap; + _interfaceMap: DefaultMap>; + constructor() { + this._fields = new MultiMap(); + this._interfaceMap = new DefaultMap>(() => new Set()); + } + + mergeExtensions(doc: DocumentNode): DocumentNode { + const sansExtensions = this.collectFieldsAndExtensions(doc); - // Merge collected extension fields into the original type/interface definition. - return visit(sansExtensions, { - ObjectTypeDefinition(t) { - const extensions = fields.get(t.name.value); - if (t.fields == null) { + this.propagateMissingFields(sansExtensions); + + // Merge collected extension fields into the original type/interface definition. + return visit(sansExtensions, { + ObjectTypeDefinition: (t) => { + const extensions = this._fields.get(t.name.value); return { ...t, fields: extensions }; - } - return { ...t, fields: [...t.fields, ...extensions] }; - }, - InterfaceTypeDefinition(t) { - const extensions = fields.get(t.name.value); - if (t.fields == null) { + }, + InterfaceTypeDefinition: (t) => { + const extensions = this._fields.get(t.name.value); return { ...t, fields: extensions }; + }, + }); + } + + // Propagate fields from interfaces to their implementing types. + // GraphQL expects all types and interfaces to explicitly define all fields, including + // those required by interfaces they implement. They way Grats works, if a field is defined + // on an interface, that field is provably present on all implementing types. + propagateMissingFields(sansExtensions: DocumentNode) { + const processedInterfaces = new Set(); + + const extendObject = (name: string): void => { + const ownFields = this._fields.get(name); + for (const iface of this._interfaceMap.get(name)) { + // First make sure the interface has been fully populated + extendInterface(iface); + const ifaceFields = this._fields.get(iface); + for (const field of ifaceFields) { + if (!ownFields.some((f) => f.name.value === field.name.value)) { + ownFields.push(field); + } + } } - return { ...t, fields: [...t.fields, ...extensions] }; - }, - }); + }; + + const extendInterface = (name: string): void => { + // Avoid duplicate processing and infinite recursion + if (processedInterfaces.has(name)) { + return; + } + // This does not _correctly_ handle recursive interfaces, but that's okay + // since some other validation pass should detect that. The main issues is + // to avoid infinite recursion leading to a stack overflow. + processedInterfaces.add(name); + const ownFields = this._fields.get(name); + for (const iface of this._interfaceMap.get(name)) { + extendInterface(iface); + const ifaceFields = this._fields.get(iface); + for (const field of ifaceFields) { + if (!ownFields.some((f) => f.name.value === field.name.value)) { + ownFields.push(field); + } + } + } + }; + + for (const doc of sansExtensions.definitions) { + switch (doc.kind) { + case "ObjectTypeDefinition": + extendObject(doc.name.value); + break; + case "InterfaceTypeDefinition": + extendInterface(doc.name.value); + break; + } + } + } + + // Collect all the fields and interfaces from the extensions and trim them from the AST. + collectFieldsAndExtensions(doc: DocumentNode): DocumentNode { + return visit(doc, { + ObjectTypeExtension: (t) => { + if (t.directives != null || t.interfaces != null) { + throw new Error("Unexpected directives or interfaces on Extension"); + } + this._fields.extend(t.name.value, t.fields); + return null; + }, + InterfaceTypeExtension: (t) => { + if (t.directives != null || t.interfaces != null) { + throw new Error("Unexpected directives or interfaces on Extension"); + } + this._fields.extend(t.name.value, t.fields); + return null; + }, + ObjectTypeDefinition: (t) => { + if (t.interfaces != null) { + for (const i of t.interfaces) { + this._interfaceMap.get(t.name.value).add(i.name.value); + } + } + this._fields.extend(t.name.value, t.fields); + return t; + }, + InterfaceTypeDefinition: (t) => { + if (t.interfaces != null) { + for (const i of t.interfaces) { + this._interfaceMap.get(t.name.value).add(i.name.value); + } + } + this._fields.extend(t.name.value, t.fields); + return t; + }, + // Grats does not create these extension types + ScalarTypeExtension(_) { + throw new Error("Unexpected ScalarTypeExtension"); + }, + EnumTypeExtension(_) { + throw new Error("Unexpected EnumTypeExtension"); + }, + SchemaExtension(_) { + throw new Error("Unexpected SchemaExtension"); + }, + }); + } } // Map a key to an array of values. From a142a2dae189730eb6a2e8a7916c5e0d0c018b59 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 14:16:54 -0800 Subject: [PATCH 2/8] Code cleanup --- src/transforms/mergeExtensions.ts | 86 ++++++++++++------------------- 1 file changed, 32 insertions(+), 54 deletions(-) diff --git a/src/transforms/mergeExtensions.ts b/src/transforms/mergeExtensions.ts index 47f2d055..e34477af 100644 --- a/src/transforms/mergeExtensions.ts +++ b/src/transforms/mergeExtensions.ts @@ -1,4 +1,9 @@ -import { DocumentNode, FieldDefinitionNode, visit } from "graphql"; +import { + DocumentNode, + FieldDefinitionNode, + NamedTypeNode, + visit, +} from "graphql"; import { DefaultMap, extend } from "../utils/helpers"; /** @@ -11,11 +16,11 @@ export function mergeExtensions(doc: DocumentNode): DocumentNode { } class ExtensionMerger { - _fields: MultiMap; + _fields: DefaultMap; _interfaceMap: DefaultMap>; constructor() { - this._fields = new MultiMap(); - this._interfaceMap = new DefaultMap>(() => new Set()); + this._fields = new DefaultMap(() => []); + this._interfaceMap = new DefaultMap(() => new Set()); } mergeExtensions(doc: DocumentNode): DocumentNode { @@ -43,7 +48,7 @@ class ExtensionMerger { propagateMissingFields(sansExtensions: DocumentNode) { const processedInterfaces = new Set(); - const extendObject = (name: string): void => { + const extendType = (name: string): void => { const ownFields = this._fields.get(name); for (const iface of this._interfaceMap.get(name)) { // First make sure the interface has been fully populated @@ -66,22 +71,13 @@ class ExtensionMerger { // since some other validation pass should detect that. The main issues is // to avoid infinite recursion leading to a stack overflow. processedInterfaces.add(name); - const ownFields = this._fields.get(name); - for (const iface of this._interfaceMap.get(name)) { - extendInterface(iface); - const ifaceFields = this._fields.get(iface); - for (const field of ifaceFields) { - if (!ownFields.some((f) => f.name.value === field.name.value)) { - ownFields.push(field); - } - } - } + extendType(name); }; for (const doc of sansExtensions.definitions) { switch (doc.kind) { case "ObjectTypeDefinition": - extendObject(doc.name.value); + extendType(doc.name.value); break; case "InterfaceTypeDefinition": extendInterface(doc.name.value); @@ -97,32 +93,24 @@ class ExtensionMerger { if (t.directives != null || t.interfaces != null) { throw new Error("Unexpected directives or interfaces on Extension"); } - this._fields.extend(t.name.value, t.fields); + this.addFields(t.name.value, t.fields); return null; }, InterfaceTypeExtension: (t) => { if (t.directives != null || t.interfaces != null) { throw new Error("Unexpected directives or interfaces on Extension"); } - this._fields.extend(t.name.value, t.fields); + this.addFields(t.name.value, t.fields); return null; }, ObjectTypeDefinition: (t) => { - if (t.interfaces != null) { - for (const i of t.interfaces) { - this._interfaceMap.get(t.name.value).add(i.name.value); - } - } - this._fields.extend(t.name.value, t.fields); + this.addInterfaces(t.name.value, t.interfaces); + this.addFields(t.name.value, t.fields); return t; }, InterfaceTypeDefinition: (t) => { - if (t.interfaces != null) { - for (const i of t.interfaces) { - this._interfaceMap.get(t.name.value).add(i.name.value); - } - } - this._fields.extend(t.name.value, t.fields); + this.addInterfaces(t.name.value, t.interfaces); + this.addFields(t.name.value, t.fields); return t; }, // Grats does not create these extension types @@ -137,34 +125,24 @@ class ExtensionMerger { }, }); } -} -// Map a key to an array of values. -class MultiMap { - private readonly map = new Map(); - - push(key: K, value: V): void { - let existing = this.map.get(key); - if (existing == null) { - existing = []; - this.map.set(key, existing); + addFields( + name: string, + fields: readonly FieldDefinitionNode[] | undefined, + ): void { + if (fields != null) { + extend(this._fields.get(name), fields); } - existing.push(value); } - extend(key: K, values?: readonly V[]): void { - if (values == null) { - return; - } - let existing = this.map.get(key); - if (existing == null) { - existing = []; - this.map.set(key, existing); + addInterfaces( + name: string, + interfaces: readonly NamedTypeNode[] | undefined, + ): void { + if (interfaces != null) { + for (const i of interfaces) { + this._interfaceMap.get(name).add(i.name.value); + } } - extend(existing, values); - } - - get(key: K): V[] { - return this.map.get(key) ?? []; } } From d73c0b6f9573f1aa27ab970aaa9c671d8d70fb11 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 15:43:20 -0800 Subject: [PATCH 3/8] Simplify adding interface field implementations to concrete types --- src/InterfaceGraph.ts | 56 ------------- .../addStringFieldToInterface.ts.expected | 2 +- ...nterfaceImplementedByInterface.ts.expected | 4 +- ...gFieldToInterfaceTwice.invalid.ts.expected | 18 ----- ...atExistsOnConcreteType.invalid.ts.expected | 40 --------- ... redefineFiledThatExistsOnConcreteType.ts} | 0 ...eFiledThatExistsOnConcreteType.ts.expected | 81 +++++++++++++++++++ ...nImplementingInterface.invalid.ts.expected | 38 --------- ...FiledThatExistsOnImplementingInterface.ts} | 0 ...tExistsOnImplementingInterface.ts.expected | 79 ++++++++++++++++++ .../interfaceFirstArgumentType.ts.expected | 2 +- src/transforms/addInterfaceFields.ts | 48 +---------- src/transforms/mergeExtensions.ts | 12 ++- .../docs/04-docblock-tags/05-interfaces.mdx | 1 + 14 files changed, 175 insertions(+), 206 deletions(-) delete mode 100644 src/InterfaceGraph.ts delete mode 100644 src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected rename src/tests/fixtures/extend_interface/{redefineFiledThatExistsOnConcreteType.invalid.ts => redefineFiledThatExistsOnConcreteType.ts} (100%) create mode 100644 src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected delete mode 100644 src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected rename src/tests/fixtures/extend_interface/{redefineFiledThatExistsOnImplementingInterface.invalid.ts => redefineFiledThatExistsOnImplementingInterface.ts} (100%) create mode 100644 src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected diff --git a/src/InterfaceGraph.ts b/src/InterfaceGraph.ts deleted file mode 100644 index 3f35bb4f..00000000 --- a/src/InterfaceGraph.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { GratsDefinitionNode } from "./GraphQLConstructor"; -import { TypeContext } from "./TypeContext"; -import { DefaultMap } from "./utils/helpers"; -import { Kind } from "graphql"; - -export type InterfaceImplementor = { kind: "TYPE" | "INTERFACE"; name: string }; -export type InterfaceMap = DefaultMap>; - -/** - * Compute a map of interfaces to the types and interfaces that implement them. - */ -export function computeInterfaceMap( - typeContext: TypeContext, - docs: GratsDefinitionNode[], -): InterfaceMap { - // For each interface definition, we need to know which types and interfaces implement it. - const graph = new DefaultMap>( - () => new Set(), - ); - - const add = (interfaceName: string, implementor: InterfaceImplementor) => { - graph.get(interfaceName).add(implementor); - }; - - for (const doc of docs) { - switch (doc.kind) { - case Kind.INTERFACE_TYPE_DEFINITION: - case Kind.INTERFACE_TYPE_EXTENSION: - for (const implementor of doc.interfaces ?? []) { - const resolved = typeContext.resolveNamedType(implementor.name); - if (resolved.kind === "ERROR") { - // We trust that these errors will be reported elsewhere. - continue; - } - add(resolved.value.value, { - kind: "INTERFACE", - name: doc.name.value, - }); - } - break; - case Kind.OBJECT_TYPE_DEFINITION: - case Kind.OBJECT_TYPE_EXTENSION: - for (const implementor of doc.interfaces ?? []) { - const resolved = typeContext.resolveNamedType(implementor.name); - if (resolved.kind === "ERROR") { - // We trust that these errors will be reported elsewhere. - continue; - } - add(resolved.value.value, { kind: "TYPE", name: doc.name.value }); - } - break; - } - } - - return graph; -} diff --git a/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected b/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected index b9998b2c..1bb89f01 100644 --- a/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected +++ b/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts.expected @@ -34,7 +34,7 @@ OUTPUT ----------------- -- SDL -- interface IPerson { - greeting: String + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterface.ts") hello: String @metadata } diff --git a/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts.expected b/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts.expected index 4ae26e0e..e868ba80 100644 --- a/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts.expected +++ b/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts.expected @@ -39,11 +39,11 @@ OUTPUT ----------------- -- SDL -- interface IPerson implements IThing { - greeting: String + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts") } interface IThing { - greeting: String + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/addStringFieldToInterfaceImplementedByInterface.ts") } type Admin implements IPerson & IThing { diff --git a/src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts.expected b/src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts.expected index 3b898e1a..1c2e6843 100644 --- a/src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts.expected +++ b/src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts.expected @@ -39,24 +39,6 @@ OUTPUT ----------------- src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts:9:17 - error: Field "IPerson.greeting" can only be defined once. -9 export function greeting(person: IPerson): string { - ~~~~~~~~ - - src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts:13:5 - 13 /** @gqlField greeting */ - ~~~~~~~~~~~~~~~~~~~ - Related location -src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts:9:17 - error: Field "Admin.greeting" can only be defined once. - -9 export function greeting(person: IPerson): string { - ~~~~~~~~ - - src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts:13:5 - 13 /** @gqlField greeting */ - ~~~~~~~~~~~~~~~~~~~ - Related location -src/tests/fixtures/extend_interface/addStringFieldToInterfaceTwice.invalid.ts:9:17 - error: Field "User.greeting" can only be defined once. - 9 export function greeting(person: IPerson): string { ~~~~~~~~ diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected deleted file mode 100644 index 9c3ecc21..00000000 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts.expected +++ /dev/null @@ -1,40 +0,0 @@ ------------------ -INPUT ------------------ -/** @gqlInterface */ -interface IPerson { - name: string; - /** @gqlField */ - hello: string; -} - -/** @gqlField */ -export function greeting(person: IPerson): string { - return `Hello ${person.name}!`; -} - -/** @gqlType */ -class User implements IPerson { - __typename: "User"; - name: string; - /** @gqlField */ - hello: string; - - /** @gqlField */ - greeting(): string { - return `Hello ${this.name}!`; - } -} - ------------------ -OUTPUT ------------------ -src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:9:17 - error: Field "User.greeting" can only be defined once. - -9 export function greeting(person: IPerson): string { - ~~~~~~~~ - - src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts:21:3 - 21 greeting(): string { - ~~~~~~~~ - Related location diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts similarity index 100% rename from src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts rename to src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected new file mode 100644 index 00000000..02c249b8 --- /dev/null +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected @@ -0,0 +1,81 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface IPerson { + name: string; + /** @gqlField */ + hello: string; +} + +/** @gqlField */ +export function greeting(person: IPerson): string { + return `Hello ${person.name}!`; +} + +/** @gqlType */ +class User implements IPerson { + __typename: "User"; + name: string; + /** @gqlField */ + hello: string; + + /** @gqlField */ + greeting(): string { + return `Hello ${this.name}!`; + } +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IPerson { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts") + hello: String @metadata +} + +type User implements IPerson { + greeting: String @metadata(argCount: 0) + hello: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const IPersonType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IPerson", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + }, + hello: { + name: "hello", + type: GraphQLString + } + }; + } + }); + const UserType: GraphQLObjectType = new GraphQLObjectType({ + name: "User", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + }, + hello: { + name: "hello", + type: GraphQLString + } + }; + }, + interfaces() { + return [IPersonType]; + } + }); + return new GraphQLSchema({ + types: [IPersonType, UserType] + }); +} diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected deleted file mode 100644 index fc811d88..00000000 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts.expected +++ /dev/null @@ -1,38 +0,0 @@ ------------------ -INPUT ------------------ -/** @gqlInterface */ -interface IPerson { - name: string; - /** @gqlField */ - hello: string; -} - -/** @gqlField */ -export function greeting(person: IPerson): string { - return `Hello ${person.name}!`; -} - -/** @gqlInterface */ -interface User extends IPerson { - __typename: "User"; - name: string; - /** @gqlField */ - hello: string; - - /** @gqlField */ - greeting(): string; -} - ------------------ -OUTPUT ------------------ -src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:9:17 - error: Field "User.greeting" can only be defined once. - -9 export function greeting(person: IPerson): string { - ~~~~~~~~ - - src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts:21:3 - 21 greeting(): string; - ~~~~~~~~ - Related location diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts similarity index 100% rename from src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts rename to src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected new file mode 100644 index 00000000..58bf4d47 --- /dev/null +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected @@ -0,0 +1,79 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface IPerson { + name: string; + /** @gqlField */ + hello: string; +} + +/** @gqlField */ +export function greeting(person: IPerson): string { + return `Hello ${person.name}!`; +} + +/** @gqlInterface */ +interface User extends IPerson { + __typename: "User"; + name: string; + /** @gqlField */ + hello: string; + + /** @gqlField */ + greeting(): string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface IPerson { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts") + hello: String @metadata +} + +interface User implements IPerson { + greeting: String @metadata(argCount: 0) + hello: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString } from "graphql"; +export function getSchema(): GraphQLSchema { + const IPersonType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IPerson", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + }, + hello: { + name: "hello", + type: GraphQLString + } + }; + } + }); + const UserType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "User", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + }, + hello: { + name: "hello", + type: GraphQLString + } + }; + }, + interfaces() { + return [IPersonType]; + } + }); + return new GraphQLSchema({ + types: [IPersonType, UserType] + }); +} diff --git a/src/tests/fixtures/extend_type/interfaceFirstArgumentType.ts.expected b/src/tests/fixtures/extend_type/interfaceFirstArgumentType.ts.expected index 4a357890..a9970d0c 100644 --- a/src/tests/fixtures/extend_type/interfaceFirstArgumentType.ts.expected +++ b/src/tests/fixtures/extend_type/interfaceFirstArgumentType.ts.expected @@ -24,7 +24,7 @@ OUTPUT -- SDL -- interface IFoo { bar: String @metadata - greeting: String + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_type/interfaceFirstArgumentType.ts") } type SomeType { diff --git a/src/transforms/addInterfaceFields.ts b/src/transforms/addInterfaceFields.ts index 6f27b4c6..823852f0 100644 --- a/src/transforms/addInterfaceFields.ts +++ b/src/transforms/addInterfaceFields.ts @@ -8,14 +8,12 @@ import { gqlRelated, } from "../utils/DiagnosticError"; import { err, ok } from "../utils/Result"; -import { InterfaceMap, computeInterfaceMap } from "../InterfaceGraph"; import { extend } from "../utils/helpers"; import { FIELD_TAG } from "../Extractor"; import { AbstractFieldDefinitionNode, GratsDefinitionNode, } from "../GraphQLConstructor"; -import { FIELD_METADATA_DIRECTIVE } from "../metadataDirectives"; /** * Grats allows you to define GraphQL fields on TypeScript interfaces using @@ -32,15 +30,9 @@ export function addInterfaceFields( const newDocs: DefinitionNode[] = []; const errors: ts.DiagnosticWithLocation[] = []; - const interfaceGraph = computeInterfaceMap(ctx, docs); - for (const doc of docs) { if (doc.kind === "AbstractFieldDefinition") { - const abstractDocResults = addAbstractFieldDefinition( - ctx, - doc, - interfaceGraph, - ); + const abstractDocResults = addAbstractFieldDefinition(ctx, doc); if (abstractDocResults.kind === "ERROR") { extend(errors, abstractDocResults.err); } else { @@ -61,7 +53,6 @@ export function addInterfaceFields( function addAbstractFieldDefinition( ctx: TypeContext, doc: AbstractFieldDefinitionNode, - interfaceGraph: InterfaceMap, ): DiagnosticsResult { const newDocs: DefinitionNode[] = []; const definitionResult = ctx.getNameDefinition(doc.onType); @@ -74,7 +65,6 @@ function addAbstractFieldDefinition( switch (nameDefinition.kind) { case "TYPE": - // Extending a type, is just adding a field to it. newDocs.push({ kind: Kind.OBJECT_TYPE_EXTENSION, name: doc.onType, @@ -83,45 +73,11 @@ function addAbstractFieldDefinition( }); break; case "INTERFACE": { - // Extending an interface is a bit more complicated. We need to add the field - // to the interface, and to each type that implements the interface. - - // The interface field definition is not executable, so we don't - // need to annotate it with the details of the implementation. - const directives = doc.field.directives?.filter((directive) => { - return directive.name.value !== FIELD_METADATA_DIRECTIVE; - }); newDocs.push({ kind: Kind.INTERFACE_TYPE_EXTENSION, name: doc.onType, - fields: [{ ...doc.field, directives }], + fields: [doc.field], }); - - for (const implementor of interfaceGraph.get(nameDefinition.name.value)) { - const name = { - kind: Kind.NAME, - value: implementor.name, - loc: doc.loc, // Bit of a lie, but I don't see a better option. - } as const; - switch (implementor.kind) { - case "TYPE": - newDocs.push({ - kind: Kind.OBJECT_TYPE_EXTENSION, - name, - fields: [doc.field], - loc: doc.loc, - }); - break; - case "INTERFACE": - newDocs.push({ - kind: Kind.INTERFACE_TYPE_EXTENSION, - name, - fields: [{ ...doc.field, directives }], - loc: doc.loc, - }); - break; - } - } break; } default: { diff --git a/src/transforms/mergeExtensions.ts b/src/transforms/mergeExtensions.ts index e34477af..7498fb68 100644 --- a/src/transforms/mergeExtensions.ts +++ b/src/transforms/mergeExtensions.ts @@ -9,6 +9,8 @@ import { DefaultMap, extend } from "../utils/helpers"; /** * Takes every example of `extend type Foo` and `extend interface Foo` and * merges them into the original type/interface definition. + * + * Also, propagates fields from interfaces to their implementing types. */ export function mergeExtensions(doc: DocumentNode): DocumentNode { const merger = new ExtensionMerger(); @@ -31,12 +33,10 @@ class ExtensionMerger { // Merge collected extension fields into the original type/interface definition. return visit(sansExtensions, { ObjectTypeDefinition: (t) => { - const extensions = this._fields.get(t.name.value); - return { ...t, fields: extensions }; + return { ...t, fields: this._fields.get(t.name.value) }; }, InterfaceTypeDefinition: (t) => { - const extensions = this._fields.get(t.name.value); - return { ...t, fields: extensions }; + return { ...t, fields: this._fields.get(t.name.value) }; }, }); } @@ -55,6 +55,10 @@ class ExtensionMerger { extendInterface(iface); const ifaceFields = this._fields.get(iface); for (const field of ifaceFields) { + // TODO: This is not very efficient. Every interface's field requires an O(n) search. + // Modeling fields a a name => field map would be more efficient but + // ends up implicitly removing duplicates, which should be reported as + // an error by later phases of the pipeline. if (!ownFields.some((f) => f.name.value === field.name.value)) { ownFields.push(field); } diff --git a/website/docs/04-docblock-tags/05-interfaces.mdx b/website/docs/04-docblock-tags/05-interfaces.mdx index e1f8efe1..a7123799 100644 --- a/website/docs/04-docblock-tags/05-interfaces.mdx +++ b/website/docs/04-docblock-tags/05-interfaces.mdx @@ -34,6 +34,7 @@ Which will generate the following GraphQL schema: --- +TODO :::note Each implementor of an interface must declare define all the fields required by the interface with `/** @gqlField */`. This means that if you have an interface that implements another interface, you must define all the fields required by both interfaces. ::: From fd059cbeb62c5ac93caa5ba599e0ea667b6783ff Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 15:46:17 -0800 Subject: [PATCH 4/8] Test transitive field addition --- ...ypeInheritsFieldFromTransitiveInterface.ts | 16 +++++++++++++++ ...tsFieldFromTransitiveInterface.ts.expected | 0 ...sFunctionFieldFromTransitiveInterfacets.ts | 20 +++++++++++++++++++ ...FieldFromTransitiveInterfacets.ts.expected | 0 4 files changed, 36 insertions(+) create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts b/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts new file mode 100644 index 00000000..ae5b1dce --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts @@ -0,0 +1,16 @@ +/** @gqlInterface */ +interface Entity { + /** @gqlField */ + name: string | null; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected new file mode 100644 index 00000000..e69de29b diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts new file mode 100644 index 00000000..227418f8 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts @@ -0,0 +1,20 @@ +/** @gqlInterface */ +interface Entity { + name: string | null; +} + +/** @gqlField */ +export function greeting(entity: Entity): string { + return `Hello, ${entity.name ?? "World"}!`; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected new file mode 100644 index 00000000..e69de29b From f7a338d81ae2de36c77f4858961bba02436d981f Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 16:00:53 -0800 Subject: [PATCH 5/8] Documentation for auto interface field propagation --- .../docs/04-docblock-tags/05-interfaces.mdx | 12 ++-- .../04-interface-override-field-impl.grats.ts | 22 ++++++ .../04-interface-override-field-impl.out | 68 +++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.grats.ts create mode 100644 website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.out diff --git a/website/docs/04-docblock-tags/05-interfaces.mdx b/website/docs/04-docblock-tags/05-interfaces.mdx index a7123799..2be32394 100644 --- a/website/docs/04-docblock-tags/05-interfaces.mdx +++ b/website/docs/04-docblock-tags/05-interfaces.mdx @@ -3,6 +3,7 @@ import InterfaceDeclaration from "!!raw-loader!./snippets/04-interface-declarati import NodeInterface from "!!raw-loader!./snippets/04-merged-interface-renaming.out"; import InterfaceImplementingInterface from "!!raw-loader!./snippets/04-interface-implement-interface.out"; import InterfaceFieldCommonImpl from "!!raw-loader!./snippets/04-interface-field-common-impl.out"; +import InterfaceOverrideFieldImpl from "!!raw-loader!./snippets/04-interface-override-field-impl.out"; # Interfaces @@ -32,12 +33,13 @@ Which will generate the following GraphQL schema: ---- +## Overriding Interface Fields -TODO -:::note -Each implementor of an interface must declare define all the fields required by the interface with `/** @gqlField */`. This means that if you have an interface that implements another interface, you must define all the fields required by both interfaces. -::: +Unlike GraphQL, when you add a field to an interface with `/** @gqlField */`, the field will be automatically be added to the GraphQL definitions of all the implementors of the interface. + +If you wish to override the field in a specific implementor, you can add a `/** @gqlField */` with the same name to the implementor. + + ## Merged Interfaces diff --git a/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.grats.ts b/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.grats.ts new file mode 100644 index 00000000..6756d804 --- /dev/null +++ b/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.grats.ts @@ -0,0 +1,22 @@ +/** @gqlInterface */ +interface Person { + /** @gqlField */ + name: string; +} + +/** @gqlType */ +class User implements Person { + __typename = "User"; + name: string; + + // highlight-start + /** + * For `User` this method will be used instead of the `name` property. + * + * @gqlField name + */ + userSpecificName(): string { + return `User: this.name`; + } + // highlight-end +} diff --git a/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.out b/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.out new file mode 100644 index 00000000..f010a419 --- /dev/null +++ b/website/docs/04-docblock-tags/snippets/04-interface-override-field-impl.out @@ -0,0 +1,68 @@ +/** @gqlInterface */ +interface Person { + /** @gqlField */ + name: string; +} + +/** @gqlType */ +class User implements Person { + __typename = "User"; + name: string; + + // highlight-start + /** + * For `User` this method will be used instead of the `name` property. + * + * @gqlField name + */ + userSpecificName(): string { + return `User: this.name`; + } + // highlight-end +} + +=== SNIP === +interface Person { + name: String +} + +type User implements Person { + """For `User` this method will be used instead of the `name` property.""" + name: String +} +=== SNIP === +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const PersonType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "Person", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const UserType: GraphQLObjectType = new GraphQLObjectType({ + name: "User", + fields() { + return { + name: { + description: "For `User` this method will be used instead of the `name` property.", + name: "name", + type: GraphQLString, + resolve(source, args, context, info) { + return source.userSpecificName(source, args, context, info); + } + } + }; + }, + interfaces() { + return [PersonType]; + } + }); + return new GraphQLSchema({ + types: [PersonType, UserType] + }); +} From 647c6bfcf86c59c2244aaba3a5ea7e975346d223 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 16:44:48 -0800 Subject: [PATCH 6/8] Add more tests --- ...eFiledThatExistsOnConcreteType.ts.expected | 2 +- ...tExistsOnImplementingInterface.ts.expected | 2 +- ...tsFieldFromTransitiveInterface.ts.expected | 81 ++++++++++++ ...onFieldFromClosestTransitiveInterfacets.ts | 32 +++++ ...omClosestTransitiveInterfacets.ts.expected | 123 ++++++++++++++++++ ...FieldFromTransitiveInterfacets.ts.expected | 89 +++++++++++++ 6 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts.expected diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected index 02c249b8..b706bb00 100644 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts.expected @@ -31,7 +31,7 @@ OUTPUT ----------------- -- SDL -- interface IPerson { - greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.invalid.ts") + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnConcreteType.ts") hello: String @metadata } diff --git a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected index 58bf4d47..902bc01c 100644 --- a/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected +++ b/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts.expected @@ -29,7 +29,7 @@ OUTPUT ----------------- -- SDL -- interface IPerson { - greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.invalid.ts") + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/extend_interface/redefineFiledThatExistsOnImplementingInterface.ts") hello: String @metadata } diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected index e69de29b..94d22287 100644 --- a/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFieldFromTransitiveInterface.ts.expected @@ -0,0 +1,81 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface Entity { + /** @gqlField */ + name: string | null; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface Entity { + name: String @metadata +} + +interface IThing implements Entity { + name: String @metadata +} + +type Doohickey implements Entity & IThing { + name: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const EntityType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "Entity", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + }, + interfaces() { + return [EntityType]; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + }, + interfaces() { + return [EntityType, IThingType]; + } + }); + return new GraphQLSchema({ + types: [EntityType, IThingType, DoohickeyType] + }); +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts new file mode 100644 index 00000000..d776a6a1 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts @@ -0,0 +1,32 @@ +/** @gqlInterface */ +interface Entity { + name: string | null; +} + +/** @gqlField */ +export function greeting(entity: Entity): string { + return `Hello, ${entity.name ?? "World"}!`; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlField greeting */ +export function iThingGreeting(iThing: IThing): string { + return `Hello, ${iThing.name ?? "IThing"}!`; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} + +// Reverse the order of the interfaces to test that the order doesn't matter +/** @gqlType */ +export class Widget implements Entity, IThing { + __typename: "Widget"; + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts.expected new file mode 100644 index 00000000..fad6ebb0 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts.expected @@ -0,0 +1,123 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface Entity { + name: string | null; +} + +/** @gqlField */ +export function greeting(entity: Entity): string { + return `Hello, ${entity.name ?? "World"}!`; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlField greeting */ +export function iThingGreeting(iThing: IThing): string { + return `Hello, ${iThing.name ?? "IThing"}!`; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} + +// Reverse the order of the interfaces to test that the order doesn't matter +/** @gqlType */ +export class Widget implements Entity, IThing { + __typename: "Widget"; + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface Entity { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts") +} + +interface IThing implements Entity { + greeting: String @metadata(argCount: 1, name: "iThingGreeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts") +} + +type Doohickey implements Entity & IThing { + greeting: String @metadata(argCount: 1, name: "iThingGreeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts") +} + +type Widget implements Entity & IThing { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets.ts") +} +-- TypeScript -- +import { iThingGreeting as doohickeyIThingGreetingResolver } from "./concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets"; +import { greeting as widgetGreetingResolver } from "./concreteTypeInheritsFunctionFieldFromClosestTransitiveInterfacets"; +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const EntityType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "Entity", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + } + }; + } + }); + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + } + }; + }, + interfaces() { + return [EntityType]; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return doohickeyIThingGreetingResolver(source); + } + } + }; + }, + interfaces() { + return [EntityType, IThingType]; + } + }); + const WidgetType: GraphQLObjectType = new GraphQLObjectType({ + name: "Widget", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return widgetGreetingResolver(source); + } + } + }; + }, + interfaces() { + return [EntityType, IThingType]; + } + }); + return new GraphQLSchema({ + types: [EntityType, IThingType, DoohickeyType, WidgetType] + }); +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected index e69de29b..4dfc9ffd 100644 --- a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts.expected @@ -0,0 +1,89 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface Entity { + name: string | null; +} + +/** @gqlField */ +export function greeting(entity: Entity): string { + return `Hello, ${entity.name ?? "World"}!`; +} + +/** @gqlInterface */ +interface IThing extends Entity { + name: string | null; +} + +/** @gqlType */ +export class Doohickey implements IThing, Entity { + __typename: "Doohickey"; + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface Entity { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts") +} + +interface IThing implements Entity { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts") +} + +type Doohickey implements Entity & IThing { + greeting: String @metadata(argCount: 1, name: "greeting", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromTransitiveInterfacets.ts") +} +-- TypeScript -- +import { greeting as doohickeyGreetingResolver } from "./concreteTypeInheritsFunctionFieldFromTransitiveInterfacets"; +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const EntityType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "Entity", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + } + }; + } + }); + const IThingType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "IThing", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString + } + }; + }, + interfaces() { + return [EntityType]; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + greeting: { + name: "greeting", + type: GraphQLString, + resolve(source) { + return doohickeyGreetingResolver(source); + } + } + }; + }, + interfaces() { + return [EntityType, IThingType]; + } + }); + return new GraphQLSchema({ + types: [EntityType, IThingType, DoohickeyType] + }); +} From d83be715a2e303accd0215fa0e4daaef1f248ddd Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 18:18:17 -0800 Subject: [PATCH 7/8] Add test showing ambiguous case --- ...itsFunctionFieldFromMultipleInterfacets.ts | 19 +++++++++++++++++++ ...onFieldFromMultipleInterfacets.ts.expected | 0 2 files changed, 19 insertions(+) create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts create mode 100644 src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts new file mode 100644 index 00000000..5d617470 --- /dev/null +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts @@ -0,0 +1,19 @@ +/** @gqlInterface */ +interface Entity { + /** @gqlField */ + name: string | null; +} + +/** @gqlInterface */ +interface NotEntity {} + +/** @gqlField */ +export function name(_: NotEntity): string | null { + return "Hello"; +} + +/** @gqlType */ +export class Doohickey implements Entity, NotEntity { + __typename: "Doohickey"; + name: string; +} diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected new file mode 100644 index 00000000..e69de29b From 5b25f5650030b8d6273a87266300fa576082c6ed Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sun, 3 Mar 2024 19:52:45 -0800 Subject: [PATCH 8/8] Add missing fixture snapshot --- ...onFieldFromMultipleInterfacets.ts.expected | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected index e69de29b..a496c069 100644 --- a/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected +++ b/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts.expected @@ -0,0 +1,81 @@ +----------------- +INPUT +----------------- +/** @gqlInterface */ +interface Entity { + /** @gqlField */ + name: string | null; +} + +/** @gqlInterface */ +interface NotEntity {} + +/** @gqlField */ +export function name(_: NotEntity): string | null { + return "Hello"; +} + +/** @gqlType */ +export class Doohickey implements Entity, NotEntity { + __typename: "Doohickey"; + name: string; +} + +----------------- +OUTPUT +----------------- +-- SDL -- +interface Entity { + name: String @metadata +} + +interface NotEntity { + name: String @metadata(argCount: 1, name: "name", tsModulePath: "grats/src/tests/fixtures/interfaces/concreteTypeInheritsFunctionFieldFromMultipleInterfacets.ts") +} + +type Doohickey implements Entity & NotEntity { + name: String @metadata +} +-- TypeScript -- +import { GraphQLSchema, GraphQLInterfaceType, GraphQLString, GraphQLObjectType } from "graphql"; +export function getSchema(): GraphQLSchema { + const EntityType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "Entity", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const NotEntityType: GraphQLInterfaceType = new GraphQLInterfaceType({ + name: "NotEntity", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + } + }); + const DoohickeyType: GraphQLObjectType = new GraphQLObjectType({ + name: "Doohickey", + fields() { + return { + name: { + name: "name", + type: GraphQLString + } + }; + }, + interfaces() { + return [EntityType, NotEntityType]; + } + }); + return new GraphQLSchema({ + types: [EntityType, NotEntityType, DoohickeyType] + }); +}