fix: support __typename in field sets (#2348)

Author: Aenimus

composition-go/index.global.js

@@ -8,8 +8,10 @@ import {
 import {
   IncompatibleMergedTypesErrorParams,
   IncompatibleParentTypeMergeErrorParams,
+  IncompatibleTypeWithProvidesErrorMessageParams,
   InvalidNamedTypeErrorParams,
   InvalidRootTypeFieldEventsDirectiveData,
+  NonExternalConditionalFieldErrorParams,
   OneOfRequiredFieldsErrorParams,
   SemanticNonNullLevelsIndexOutOfBoundsErrorParams,
   SemanticNonNullLevelsNonNullErrorParams,
@@ -25,13 +27,15 @@ import {
   INTERFACE,
   LEVELS,
   LITERAL_NEW_LINE,
+  LITERAL_PERIOD,
   NOT_UPPER,
   OR_UPPER,
   QUOTATION_JOIN,
   SUBSCRIPTION_FIELD_CONDITION,
   SUBSCRIPTION_FILTER,
   SUBSCRIPTION_FILTER_CONDITION,
   SUBSCRIPTION_FILTER_VALUE,
+  TYPENAME,
   UNION,
   VALUES,
 } from '../utils/string-constants';
@@ -684,9 +688,13 @@ export function invalidConfigurationDataErrorMessage(typeName: string, fieldName
   );
 }
 
-export function incompatibleTypeWithProvidesErrorMessage(fieldCoords: string, responseType: string): string {
+export function incompatibleTypeWithProvidesErrorMessage({
+  fieldCoords,
+  responseType,
+  subgraphName,
+}: IncompatibleTypeWithProvidesErrorMessageParams): string {
   return (
-    ` A "@provides" directive is declared on field "${fieldCoords}".\n` +
+    ` A "@provides" directive is declared on field "${fieldCoords}" in subgraph "${subgraphName}".\n` +
     ` However, the response type "${responseType}" is not an Object nor Interface.`
   );
 }
@@ -1472,19 +1480,23 @@ export function externalInterfaceFieldsError(typeName: string, fieldNames: Array
   );
 }
 
-export function nonExternalConditionalFieldError(
-  directiveCoords: string,
-  subgraphName: string,
-  targetCoords: string,
-  fieldSet: string,
-  fieldSetDirectiveName: string,
-): Error {
+export function nonExternalConditionalFieldError({
+  directiveCoords,
+  fieldSet,
+  directiveName,
+  subgraphName,
+  targetCoords,
+}: NonExternalConditionalFieldErrorParams): Error {
+  const segments = targetCoords.split(LITERAL_PERIOD);
+  const isTypeName = segments[segments.length - 1] === TYPENAME;
   return new Error(
-    `The field "${directiveCoords}" in subgraph "${subgraphName}" defines a "@${fieldSetDirectiveName}"` +
+    `The field "${directiveCoords}" in subgraph "${subgraphName}" defines a "@${directiveName}"` +
       ` directive with the following field set:\n "${fieldSet}".` +
-      `\nHowever, neither the field "${targetCoords}" nor any of its field set ancestors are declared "@external".` +
+      (isTypeName
+        ? `\nHowever, none of the field set ancestors of "__typename" is declared "@external".`
+        : `\nHowever, neither the field "${targetCoords}" nor any of its field set ancestors are declared "@external".`) +
       `\nConsequently, "${targetCoords}" is already provided by subgraph "${subgraphName}" and should not form part of` +
-      ` a "@${fieldSetDirectiveName}" directive field set.`,
+      ` a "@${directiveName}" directive field set.`,
   );
 }
 
@@ -1586,6 +1598,13 @@ export function invalidDirectiveDefinitionError(directiveName: string, errorMess
   );
 }
 
+export function typeNameAlreadyProvidedErrorMessage(fieldCoords: string, subgraphName: string): string {
+  return (
+    ` The field "${fieldCoords}" is unconditionally provided by subgraph "${subgraphName}" and should not form` +
+    ` part of any "@provides" field set.`
+  );
+}
+
 export function fieldAlreadyProvidedErrorMessage(
   fieldCoords: string,
   subgraphName: string,

composition/src/errors/errors.ts

@@ -1,9 +1,9 @@
 import { FieldData, InputValueData, ParentDefinitionData } from '../schema-building/types';
-import { FieldName, SubgraphName, TypeName } from '../types/types';
+import { DirectiveName, FieldName, SubgraphName, TypeName } from '../types/types';
 
 export type InvalidRootTypeFieldEventsDirectiveData = {
   definesDirectives: boolean;
-  invalidDirectiveNames: string[];
+  invalidDirectiveNames: Array<string>;
 };
 
 export type IncompatibleMergedTypesErrorParams = {
@@ -40,3 +40,17 @@ export type IncompatibleParentTypeMergeErrorParams = {
   incomingSubgraphName: SubgraphName;
   incomingNodeType?: string;
 };
+
+export type IncompatibleTypeWithProvidesErrorMessageParams = {
+  fieldCoords: string;
+  responseType: TypeName;
+  subgraphName: SubgraphName;
+};
+
+export type NonExternalConditionalFieldErrorParams = {
+  directiveCoords: string;
+  fieldSet: string;
+  directiveName: DirectiveName;
+  subgraphName: SubgraphName;
+  targetCoords: string;
+};

composition/src/errors/types.ts

@@ -86,6 +86,7 @@ export const LINK_PURPOSE = 'link__Purpose';
 export const LIST = 'list';
 export const LITERAL_SPACE = ' ';
 export const LITERAL_NEW_LINE = '\n';
+export const LITERAL_PERIOD = '.';
 export const NUMBER = 'number';
 export const MUTATION = 'Mutation';
 export const MUTATION_UPPER = 'MUTATION';
@@ -110,7 +111,6 @@ export const OVERRIDE = 'override';
 export const PARENT_DEFINITION_DATA = 'parentDefinitionDataByTypeName';
 export const PARENT_DEFINITION_DATA_MAP = 'parentDefinitionDataByParentTypeName';
 export const PARENT_EXTENSION_DATA_MAP = 'parentExtensionDataByParentTypeName';
-export const PERIOD = '.';
 export const PROVIDER_ID = 'providerId';
 export const PROVIDES = 'provides';
 export const PUBLISH = 'publish';
@@ -153,6 +153,7 @@ export const SUCCESS = 'success';
 export const TAG = 'tag';
 export const TOPIC = 'topic';
 export const TOPICS = 'topics';
+export const TYPENAME = '__typename';
 export const UNION = 'Union';
 export const UNION_UPPER = 'UNION';
 export const URL_LOWER = 'url';

composition/src/utils/string-constants.ts

@@ -205,7 +205,7 @@ import {
   ONE_OF,
   OR_UPPER,
   PARENT_DEFINITION_DATA,
-  PERIOD,
+  LITERAL_PERIOD,
   QUERY,
   REQUIRES_SCOPES,
   SEMANTIC_NON_NULL,
@@ -598,7 +598,7 @@ export class FederationFactory {
 
   generateTagData() {
     for (const [path, tagNames] of this.tagNamesByCoords) {
-      const paths = path.split(PERIOD);
+      const paths = path.split(LITERAL_PERIOD);
       if (paths.length < 1) {
         continue;
       }
@@ -1482,7 +1482,7 @@ export class FederationFactory {
 
   handleDisparateFieldNamedTypes() {
     for (const [fieldCoordinates, subgraphNamesByNamedTypeName] of this.subgraphNamesByNamedTypeNameByFieldCoords) {
-      const coordinates = fieldCoordinates.split(PERIOD);
+      const coordinates = fieldCoordinates.split(LITERAL_PERIOD);
       if (coordinates.length !== 2) {
         continue;
       }
@@ -2241,7 +2241,7 @@ export class FederationFactory {
       }
       for (const coords of fieldCoords) {
         // The coords should all be exactly <parentTypeName>.<fieldName>
-        const segments = coords.split(PERIOD);
+        const segments = coords.split(LITERAL_PERIOD);
         switch (segments.length) {
           case 2: {
             const parentAuthData = getValueOrDefault(this.authorizationDataByParentTypeName, segments[0], () =>
@@ -2348,7 +2348,7 @@ export class FederationFactory {
       return false;
     }
     const coordinates = path.split(LEFT_PARENTHESIS)[0];
-    const segments = coordinates.split(PERIOD);
+    const segments = coordinates.split(LITERAL_PERIOD);
     let segment = segments[0];
     for (let i = 0; i < segments.length; i++) {
       if (this.inaccessibleCoords.has(segment)) {
@@ -2399,7 +2399,7 @@ export class FederationFactory {
     directiveSubgraphName: string,
     fieldErrorMessages: Array<string>,
   ): string[] {
-    const paths = conditionFieldPath.split(PERIOD);
+    const paths = conditionFieldPath.split(LITERAL_PERIOD);
     if (paths.length < 1) {
       fieldErrorMessages.push(
         invalidSubscriptionFieldConditionFieldPathErrorMessage(inputFieldPath, conditionFieldPath),

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

@@ -23,6 +23,7 @@ import { ContractTagOptions } from '../../federation/types';
 import { getOrThrowError, getValueOrDefault } from '../../utils/utils';
 import { KeyFieldSetData } from '../normalization/types';
 import { SubgraphName, TypeName } from '../../types/types';
+import { TYPENAME } from '../../utils/string-constants';
 
 export type FederationFactoryParams = {
   authorizationDataByParentTypeName: Map<TypeName, AuthorizationData>;
@@ -137,6 +138,9 @@ export function validateImplicitFieldSets({
             return BREAK;
           }
           const fieldName = node.name.value;
+          if (fieldName === TYPENAME) {
+            return;
+          }
           const fieldData = parentData.fieldDataByName.get(fieldName);
           // undefined if the field does not exist on the parent
           if (!fieldData || fieldData.argumentDataByName.size || definedFields[currentDepth].has(fieldName)) {