Skip to content

Commit

Permalink
fix: strip @external fields in interface extensions (apollographql/ap…
Browse files Browse the repository at this point in the history
…ollo-server#2848)

Before this change, including an external field in an interface
extension would fail composition in the gateway with an error:

```
Field "Product.id" already exists in the schema. It cannot also be defined in this type extension.
```
Apollo-Orig-Commit-AS: apollographql/apollo-server@c883224
  • Loading branch information
lennyburdette authored and JakeDawkins committed Jun 17, 2019
1 parent 4f4e77f commit 4345807
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 26 deletions.
11 changes: 11 additions & 0 deletions federation-js/src/composition/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ describe('Composition utility functions', () => {
type Mutation {
updateProduct: Product
}
extend interface Account @key(fields: "id") {
id: ID! @external
}
`;

const {
Expand All @@ -37,6 +41,8 @@ describe('Composition utility functions', () => {
type Mutation {
updateProduct: Product
}
extend interface Account @key(fields: "id")
`);

expect(strippedFields).toMatchInlineSnapshot(`
Expand All @@ -46,6 +52,11 @@ describe('Composition utility functions', () => {
"parentTypeName": "Product",
"serviceName": "serviceA",
},
Object {
"field": id: ID! @external,
"parentTypeName": "Account",
"serviceName": "serviceA",
},
]
`);
});
Expand Down
75 changes: 49 additions & 26 deletions federation-js/src/composition/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'apollo-server-env';
import {
ObjectTypeDefinitionNode,
InterfaceTypeExtensionNode,
FieldDefinitionNode,
Kind,
StringValueNode,
Expand Down Expand Up @@ -45,7 +46,10 @@ export function mapFieldNamesToServiceName<Node extends { name: NameNode }>(

export function findDirectivesOnTypeOrField(
node: Maybe<
ObjectTypeDefinitionNode | ObjectTypeExtensionNode | FieldDefinitionNode
| ObjectTypeDefinitionNode
| ObjectTypeExtensionNode
| FieldDefinitionNode
| InterfaceTypeExtensionNode
>,
directiveName: string,
) {
Expand All @@ -66,36 +70,55 @@ export function stripExternalFieldsFromTypeDefs(
const strippedFields: ExternalFieldDefinition[] = [];

const typeDefsWithoutExternalFields = visit(typeDefs, {
ObjectTypeExtension(node) {
let fields = node.fields;
if (fields) {
fields = fields.filter(field => {
const externalDirectives = findDirectivesOnTypeOrField(
field,
'external',
);

if (externalDirectives.length > 0) {
strippedFields.push({
field,
parentTypeName: node.name.value,
serviceName,
});
return false;
}
return true;
});
}
return {
...node,
fields,
};
},
ObjectTypeExtension: removeExternalFieldsFromExtensionVisitor(
strippedFields,
serviceName,
),
InterfaceTypeExtension: removeExternalFieldsFromExtensionVisitor(
strippedFields,
serviceName,
),
}) as DocumentNode;

return { typeDefsWithoutExternalFields, strippedFields };
}

/**
* Returns a closure that strips fields marked with `@external` and adds them
* to an array.
* @param collector
* @param serviceName
*/
function removeExternalFieldsFromExtensionVisitor<
T extends InterfaceTypeExtensionNode | ObjectTypeExtensionNode
>(collector: ExternalFieldDefinition[], serviceName: string) {
return (node: T) => {
let fields = node.fields;
if (fields) {
fields = fields.filter(field => {
const externalDirectives = findDirectivesOnTypeOrField(
field,
'external',
);

if (externalDirectives.length > 0) {
collector.push({
field,
parentTypeName: node.name.value,
serviceName,
});
return false;
}
return true;
});
}
return {
...node,
fields,
};
};
}

export function parseSelections(source: string) {
return (parse(`query { ${source} }`)
.definitions[0] as OperationDefinitionNode).selectionSet.selections;
Expand Down

0 comments on commit 4345807

Please sign in to comment.