Skip to content

Commit 6e28b2c

Browse files
AenimusAenimus
committed
chore: improve incompatible types error
1 parent e7c05cb commit 6e28b2c

File tree

7 files changed

+234
-85
lines changed

7 files changed

+234
-85
lines changed

‎composition-go/index.global.js‎

Lines changed: 37 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎composition/src/errors/errors.ts‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from '../schema-building/types';
88
import {
99
IncompatibleMergedTypesErrorParams,
10+
IncompatibleParentKindMergeErrorParams,
1011
InvalidNamedTypeErrorParams,
1112
InvalidRootTypeFieldEventsDirectiveData,
1213
OneOfRequiredFieldsErrorParams,
@@ -39,7 +40,7 @@ import { getEntriesNotInHashSet, getOrThrowError, kindToNodeType, numberToOrdina
3940
import { ImplementationErrors, InvalidEntityInterface, InvalidRequiredInputValueData } from '../utils/types';
4041
import { isFieldData } from '../schema-building/utils';
4142
import { printTypeNode } from '@graphql-tools/merge';
42-
import { NodeType, TypeName } from '../types/types';
43+
import { NodeType, SubgraphName, TypeName } from '../types/types';
4344

4445
export const minimumSubgraphRequirementError = new Error('At least one subgraph is required for federation.');
4546

@@ -338,20 +339,21 @@ export function unexpectedEdgeFatalError(typeName: string, edgeNames: Array<stri
338339
);
339340
}
340341

341-
export function incompatibleParentKindMergeError(
342-
parentTypeName: string,
343-
expectedTypeString: string,
344-
actualTypeString: string,
345-
): Error {
346-
return new Error(
347-
` When merging types, expected "${parentTypeName}" to be type "${expectedTypeString}" but received "${actualTypeString}".`,
348-
);
349-
}
342+
const interfaceObject = `"Interface Object" (an "Object" type that also defines the "@interfaceObject" directive)`;
350343

351-
export function fieldTypeMergeFatalError(fieldName: string) {
344+
export function incompatibleParentTypeMergeError({
345+
existingData,
346+
incomingNodeType,
347+
incomingSubgraphName,
348+
}: IncompatibleParentKindMergeErrorParams): Error {
349+
const existingSubgraphNames = new Array<SubgraphName>(...existingData.subgraphNames);
350+
const nodeType = incomingNodeType ? `"${incomingNodeType}"` : interfaceObject;
352351
return new Error(
353-
`Fatal: Unsuccessfully merged the cross-subgraph types of field "${fieldName}"` +
354-
` without producing a type error object.`,
352+
` "${existingData.name}" is defined using incompatible types across subgraphs.` +
353+
` It is defined as type "${kindToNodeType(existingData.kind)}" in subgraph` +
354+
(existingSubgraphNames.length > 1 ? 's' : '') +
355+
` "${existingSubgraphNames.join(QUOTATION_JOIN)}" but type ${nodeType} in subgraph` +
356+
` "${incomingSubgraphName}".`,
355357
);
356358
}
357359

@@ -378,7 +380,7 @@ export function unexpectedParentKindForChildError(
378380
childTypeString: string,
379381
): Error {
380382
return new Error(
381-
` Expected "${parentTypeName}" to be type ${expectedTypeString} but received "${actualTypeString}"` +
383+
` Expected "${parentTypeName}" to be type "${expectedTypeString}" but received "${actualTypeString}"` +
382384
` when handling child "${childName}" of type "${childTypeString}".`,
383385
);
384386
}

‎composition/src/errors/types.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { FieldData, InputValueData, ParentDefinitionData } from '../schema-building/types';
2-
import { FieldName, TypeName } from '../types/types';
2+
import { FieldName, SubgraphName, TypeName } from '../types/types';
33

44
export type InvalidRootTypeFieldEventsDirectiveData = {
55
definesDirectives: boolean;
@@ -34,3 +34,9 @@ export type OneOfRequiredFieldsErrorParams = {
3434
requiredFieldNames: Array<FieldName>;
3535
typeName: TypeName;
3636
};
37+
38+
export type IncompatibleParentKindMergeErrorParams = {
39+
existingData: ParentDefinitionData;
40+
incomingSubgraphName: SubgraphName;
41+
incomingNodeType?: string;
42+
};

‎composition/src/v1/federation/federation-factory.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
incompatibleFederatedFieldNamedTypeError,
3434
incompatibleMergedTypesError,
3535
incompatibleParentKindFatalError,
36-
incompatibleParentKindMergeError,
36+
incompatibleParentTypeMergeError,
3737
incompatibleSharedEnumError,
3838
invalidFieldShareabilityError,
3939
invalidImplementedTypeError,
@@ -1207,7 +1207,7 @@ export class FederationFactory {
12071207
return existingData;
12081208
}
12091209

1210-
upsertParentDefinitionData(incomingData: ParentDefinitionData, subgraphName: string) {
1210+
upsertParentDefinitionData(incomingData: ParentDefinitionData, subgraphName: SubgraphName) {
12111211
const entityInterfaceData = this.entityInterfaceFederationDataByTypeName.get(incomingData.name);
12121212
const existingData = this.parentDefinitionDataByTypeName.get(incomingData.name);
12131213
const targetData = this.getParentTargetData({ existingData, incomingData });
@@ -1217,6 +1217,15 @@ export class FederationFactory {
12171217
this.inaccessibleCoords.add(targetData.name);
12181218
}
12191219
if (entityInterfaceData && entityInterfaceData.interfaceObjectSubgraphNames.has(subgraphName)) {
1220+
if (existingData && existingData.kind !== Kind.INTERFACE_TYPE_DEFINITION) {
1221+
this.errors.push(
1222+
incompatibleParentTypeMergeError({
1223+
existingData,
1224+
incomingSubgraphName: subgraphName,
1225+
}),
1226+
);
1227+
return;
1228+
}
12201229
targetData.kind = Kind.INTERFACE_TYPE_DEFINITION;
12211230
targetData.node.kind = Kind.INTERFACE_TYPE_DEFINITION;
12221231
}
@@ -1232,11 +1241,11 @@ export class FederationFactory {
12321241
incomingData.kind !== Kind.OBJECT_TYPE_DEFINITION
12331242
) {
12341243
this.errors.push(
1235-
incompatibleParentKindMergeError(
1236-
targetData.name,
1237-
kindToNodeType(targetData.kind),
1238-
kindToNodeType(incomingData.kind),
1239-
),
1244+
incompatibleParentTypeMergeError({
1245+
existingData: targetData,
1246+
incomingNodeType: kindToNodeType(incomingData.kind),
1247+
incomingSubgraphName: subgraphName,
1248+
}),
12401249
);
12411250
return;
12421251
}

‎composition/tests/v1/entity-interface.test.ts‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,22 @@ import {
22
ConfigurationData,
33
EntityInterfaceFederationData,
44
EntityInterfaceSubgraphData,
5+
incompatibleParentTypeMergeError,
56
INTERFACE,
7+
InterfaceDefinitionData,
68
InvalidEntityInterface,
9+
OBJECT,
10+
ObjectDefinitionData,
711
ROUTER_COMPATIBILITY_VERSION_ONE,
812
SimpleFieldData,
913
Subgraph,
14+
SubgraphName,
1015
undefinedEntityInterfaceImplementationsError,
1116
} from '../../src';
1217
import { describe, expect, test } from 'vitest';
1318
import { versionOneRouterDefinitions, versionTwoRouterDefinitions } from './utils/utils';
1419

15-
import { parse } from 'graphql';
20+
import { Kind, parse } from 'graphql';
1621
import {
1722
federateSubgraphsFailure,
1823
federateSubgraphsSuccess,
@@ -256,7 +261,7 @@ describe('Entity Interface Tests', () => {
256261
);
257262
});
258263

259-
test('that @interfaceObject works correctly with implicit key checks #1.2', () => {
264+
test('that @interfaceObject works correctly with implicit key checks #1.2', () => {
260265
const result = federateSubgraphsSuccess([subgraphJ, subgraphI], ROUTER_COMPATIBILITY_VERSION_ONE);
261266
expect(result.success).toBe(true);
262267
expect(schemaToSortedNormalizedString(result.federatedGraphSchema)).toBe(
@@ -300,6 +305,44 @@ describe('Entity Interface Tests', () => {
300305
);
301306
});
302307

308+
test('that error is returned if an entity Interface is defined as a regular Object type #1', () => {
309+
const { errors } = federateSubgraphsFailure([kaaa, kaab, kaac], ROUTER_COMPATIBILITY_VERSION_ONE);
310+
expect(errors).toHaveLength(1);
311+
const existingData = {
312+
kind: Kind.INTERFACE_TYPE_DEFINITION,
313+
name: INTERFACE,
314+
subgraphNames: new Set<SubgraphName>([kaaa.name, kaab.name]),
315+
} as InterfaceDefinitionData;
316+
expect(errors).toStrictEqual([
317+
incompatibleParentTypeMergeError({
318+
existingData,
319+
incomingNodeType: OBJECT,
320+
incomingSubgraphName: kaac.name,
321+
}),
322+
]);
323+
});
324+
325+
test('that error is returned if an entity Interface is defined as a regular Object type #2', () => {
326+
const { errors } = federateSubgraphsFailure([kaac, kaab, kaaa], ROUTER_COMPATIBILITY_VERSION_ONE);
327+
expect(errors).toHaveLength(2);
328+
const existingData = {
329+
kind: Kind.OBJECT_TYPE_DEFINITION,
330+
name: INTERFACE,
331+
subgraphNames: new Set<SubgraphName>([kaac.name]),
332+
} as ObjectDefinitionData;
333+
expect(errors).toStrictEqual([
334+
incompatibleParentTypeMergeError({
335+
existingData,
336+
incomingSubgraphName: kaab.name,
337+
}),
338+
incompatibleParentTypeMergeError({
339+
existingData,
340+
incomingNodeType: INTERFACE,
341+
incomingSubgraphName: kaaa.name,
342+
}),
343+
]);
344+
});
345+
303346
test.skip('that an error is returned if a type declared with @interfaceObject is not an interface in other subgraphs', () => {});
304347

305348
test.skip('that an error is returned if a type declared with @interfaceObject is not an entity', () => {});
@@ -504,3 +547,37 @@ const subgraphJ: Subgraph = {
504547
}
505548
`),
506549
};
550+
551+
const kaaa: Subgraph = {
552+
name: 'kaaa',
553+
url: '',
554+
definitions: parse(`
555+
interface Interface @key(fields: "id", resolvable: false) {
556+
id: ID!
557+
}
558+
559+
type Query {
560+
a: ID
561+
}
562+
`),
563+
};
564+
565+
const kaab: Subgraph = {
566+
name: 'kaab',
567+
url: '',
568+
definitions: parse(`
569+
type Interface @key(fields: "id", resolvable: false) @interfaceObject {
570+
id: ID!
571+
}
572+
`),
573+
};
574+
575+
const kaac: Subgraph = {
576+
name: 'kaac',
577+
url: '',
578+
definitions: parse(`
579+
type Interface @key(fields: "id", resolvable: false) {
580+
id: ID!
581+
}
582+
`),
583+
};

‎composition/tests/v1/federation-factory.test.ts‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import {
2-
incompatibleParentKindMergeError,
2+
incompatibleParentTypeMergeError,
33
INPUT_OBJECT,
4+
InputObjectDefinitionData,
45
invalidSubgraphNamesError,
56
noBaseDefinitionForExtensionError,
67
noQueryRootTypeError,
78
OBJECT,
9+
ObjectDefinitionData,
810
parse,
911
ROUTER_COMPATIBILITY_VERSION_ONE,
1012
SCALAR,
13+
ScalarDefinitionData,
1114
Subgraph,
15+
SubgraphName,
1216
} from '../../src';
1317
import { describe, expect, test } from 'vitest';
1418
import {
@@ -28,6 +32,7 @@ import {
2832
normalizeString,
2933
schemaToSortedNormalizedString,
3034
} from '../utils/utils';
35+
import { Kind } from 'graphql';
3136

3237
// @ts-ignore
3338
const __filename = fileURLToPath(import.meta.url);
@@ -880,26 +885,70 @@ describe('FederationFactory tests', () => {
880885
test('that an error is returned when merging incompatible types #1.1', () => {
881886
const result = federateSubgraphsFailure([subgraphR, subgraphS], ROUTER_COMPATIBILITY_VERSION_ONE);
882887
expect(result.errors).toHaveLength(1);
883-
expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, SCALAR, OBJECT));
888+
const existingData = {
889+
kind: Kind.SCALAR_TYPE_DEFINITION,
890+
name: OBJECT,
891+
subgraphNames: new Set<SubgraphName>([subgraphR.name]),
892+
} as ScalarDefinitionData;
893+
expect(result.errors).toStrictEqual([
894+
incompatibleParentTypeMergeError({
895+
existingData,
896+
incomingNodeType: OBJECT,
897+
incomingSubgraphName: subgraphS.name,
898+
}),
899+
]);
884900
});
885901

886902
test('that an error is returned when merging incompatible types #1.2', () => {
887903
const result = federateSubgraphsFailure([subgraphS, subgraphR], ROUTER_COMPATIBILITY_VERSION_ONE);
888904
expect(result.errors).toHaveLength(1);
889-
expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, OBJECT, SCALAR));
905+
const existingData = {
906+
kind: Kind.OBJECT_TYPE_DEFINITION,
907+
name: OBJECT,
908+
subgraphNames: new Set<SubgraphName>([subgraphS.name]),
909+
} as ObjectDefinitionData;
910+
expect(result.errors).toStrictEqual([
911+
incompatibleParentTypeMergeError({
912+
existingData,
913+
incomingNodeType: SCALAR,
914+
incomingSubgraphName: subgraphR.name,
915+
}),
916+
]);
890917
});
891918

892919
test('that an error is returned when merging an object extension orphan with an incompatible base type #1.1', () => {
893920
const result = federateSubgraphsFailure([subgraphT, subgraphU], ROUTER_COMPATIBILITY_VERSION_ONE);
894921
expect(result.errors).toHaveLength(2);
895-
expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, OBJECT, INPUT_OBJECT));
896-
expect(result.errors[1]).toStrictEqual(noBaseDefinitionForExtensionError(OBJECT, OBJECT));
922+
const existingData = {
923+
kind: Kind.OBJECT_TYPE_DEFINITION,
924+
name: OBJECT,
925+
subgraphNames: new Set<SubgraphName>([subgraphT.name]),
926+
} as ObjectDefinitionData;
927+
expect(result.errors).toStrictEqual([
928+
incompatibleParentTypeMergeError({
929+
existingData,
930+
incomingNodeType: INPUT_OBJECT,
931+
incomingSubgraphName: subgraphU.name,
932+
}),
933+
noBaseDefinitionForExtensionError(OBJECT, OBJECT),
934+
]);
897935
});
898936

899937
test('that an error is returned when merging an object extension orphan with an incompatible base type #1.2', () => {
900938
const result = federateSubgraphsFailure([subgraphU, subgraphT], ROUTER_COMPATIBILITY_VERSION_ONE);
901939
expect(result.errors).toHaveLength(1);
902-
expect(result.errors[0]).toStrictEqual(incompatibleParentKindMergeError(OBJECT, INPUT_OBJECT, OBJECT));
940+
const existingData = {
941+
kind: Kind.INPUT_OBJECT_TYPE_DEFINITION,
942+
name: OBJECT,
943+
subgraphNames: new Set<SubgraphName>([subgraphU.name]),
944+
} as InputObjectDefinitionData;
945+
expect(result.errors).toStrictEqual([
946+
incompatibleParentTypeMergeError({
947+
existingData,
948+
incomingNodeType: OBJECT,
949+
incomingSubgraphName: subgraphT.name,
950+
}),
951+
]);
903952
});
904953

905954
test('that renaming a root type also renames field return types of the same type #1.1', () => {

0 commit comments

Comments
 (0)