Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge default array arguments for external types #3096

Open
wants to merge 7 commits into
base: next
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/sixty-plums-enjoy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/composition": patch
---

Allow merging external types when using arrays as default arguments.
41 changes: 41 additions & 0 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,47 @@ describe('composition', () => {
`);
});

it('merges default arguments when they are arrays', () => {
const subgraphA = {
name: 'subgraph-a',
typeDefs: gql`
type Query {
a: A @shareable
}

type A @key(fields: "id") {
id: ID
get(ids: [ID] = []): [B] @external
req: Int @requires(fields: "get { __typename }")
}

type B @key(fields: "id", resolvable: false) {
id: ID
}
`
};
const subgraphB = {
name: 'subgraph-b',
typeDefs: gql`
type Query {
a: A @shareable
}

type A @key(fields: "id") {
id: ID
get(ids: [ID] = []): [B]
}

type B @key(fields: "id") {
id: ID
}
`
};

const result = composeAsFed2Subgraphs([subgraphA, subgraphB]);
assertCompositionSuccess(result);
});

describe('merging of type references', () => {
describe('for field types', () => {
it('errors on incompatible types', () => {
Expand Down
4 changes: 3 additions & 1 deletion composition-js/src/merging/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1794,7 +1794,9 @@ class Merger {
if (!sameType(destArg.type!, arg.type!) && !this.isStrictSubtype(arg.type!, destArg.type!)) {
invalidArgsTypes.add(name);
}
if (destArg.defaultValue !== arg.defaultValue) {
const defaultIsArray = Array.isArray(destArg.defaultValue) && Array.isArray(arg.defaultValue);
tninesling marked this conversation as resolved.
Show resolved Hide resolved
const defaultArgumentsAreEqual = defaultIsArray ? arrayEquals(destArg.defaultValue, arg.defaultValue) : destArg.defaultValue === arg.defaultValue;
if (!defaultArgumentsAreEqual) {
invalidArgsDefaults.add(name);
}
}
Expand Down
3 changes: 1 addition & 2 deletions composition-js/src/merging/reporter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addSubgraphToASTNode, assert, ErrorCodeDefinition, joinStrings, MultiMap, NamedSchemaElement, printSubgraphNames, SubgraphASTNode } from '@apollo/federation-internals';
import { addSubgraphToASTNode, ErrorCodeDefinition, joinStrings, MultiMap, NamedSchemaElement, printSubgraphNames, SubgraphASTNode } from '@apollo/federation-internals';
import { ASTNode, GraphQLError } from 'graphql';
import { CompositionHint, HintCodeDefinition } from '../hints';
import { Sources } from './merge';
Expand Down Expand Up @@ -182,7 +182,6 @@ export class MismatchReporter {
}
}
const supergraphMismatch = (supergraphElement && mismatchAccessor(supergraphElement, true)) ?? '';
assert(distributionMap.size > 1, () => `Should not have been called for ${supergraphElement}`);
const distribution = [];
// We always add the "supergraph" first (proper formatting of hints rely on this in particular).
const subgraphsLikeSupergraph = distributionMap.get(supergraphMismatch);
Expand Down