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.

Sign up for GitHub

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

Jump to bottom

Insert empty Subscription type definition #212

Open
wants to merge 4 commits into
base: version-0.x
Choose a base branch
from
Open

Insert empty Subscription type definition #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb27f6c
Add failing tests for Subscription type
Chase-C May 22, 2020
ca1fc38
Insert check for existance of Subscription definition
Chase-C May 22, 2020
51a01c6
Merge remote-tracking branch 'origin/main' into apollo-server-import/…
abernix Sep 17, 2020
c89e0b3
Merge remote-tracking branch 'origin/main' into apollo-server-import/…
abernix Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions federation-js/src/composition/__tests__/compose.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,78 @@ describe('composeServices', () => {
`);
});

it('allows extension of the Subscription type with no base type definition', () => {
const serviceA = {
typeDefs: gql`
extend type Subscription {
postAdded: Post
}

type Post {
author: String!
comment: String!
}
`,
name: 'serviceA',
};

const serviceB = {
typeDefs: gql`
extend type Subscription {
postRemoved: ID
}
`,
name: 'serviceB',
};

const { schema, errors } = composeServices([serviceA, serviceB]);
expect(errors).toHaveLength(0);
expect(schema).toBeDefined();

expect(schema.getType('Subscription')).toMatchInlineSnapshot(`
type Subscription {
postAdded: Post
postRemoved: ID
}
`);
});

it('treats root Subscriptions type definition as an extension, not base definitions', () => {
const serviceA = {
typeDefs: gql`
type Subscription {
postAdded: Post
}

type Post {
author: String!
comment: String!
}
`,
name: 'serviceA',
};

const serviceB = {
typeDefs: gql`
extend type Subscription {
postRemoved: ID
}
`,
name: 'serviceB',
};

const { schema, errors } = composeServices([serviceA, serviceB]);
expect(errors).toHaveLength(0);
expect(schema).toBeDefined();

expect(schema.getType('Subscription')).toMatchInlineSnapshot(`
type Subscription {
postAdded: Post
postRemoved: ID
}
`);
});

// TODO: not sure what to do here. Haven't looked into it yet :)
it.skip('works with custom root types', () => {});
});
Expand Down
8 changes: 8 additions & 0 deletions federation-js/src/composition/compose.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ const EmptyMutationDefinition = {
fields: [],
serviceName: null,
};
const EmptySubscriptionDefinition = {
kind: Kind.OBJECT_TYPE_DEFINITION,
name: { kind: Kind.NAME, value: 'Subscription' },
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a constant here?
Like this:

name: { kind: Kind.NAME, value: defaultRootOperationNameLookup.subscription },

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Chase-C can I help you with this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be my guest. I tried making the change you suggested, but I'm not sure it's possible to push new commits to this branch without creating an entirely new PR (this one was migrated from the apollo-server project).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried here, and really need create a new PR with fork. Do you want to create there? Or do I create here?

fields: [],
serviceName: null,
};

// Map of all type definitions to eventually be passed to extendSchema
interface TypeDefinitionsMap {
Expand Down Expand Up @@ -326,6 +332,8 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) {
typeDefinitionsMap.Query = [EmptyQueryDefinition];
if (typeExtensionsMap.Mutation && !typeDefinitionsMap.Mutation)
typeDefinitionsMap.Mutation = [EmptyMutationDefinition];
if (typeExtensionsMap.Subscription && !typeDefinitionsMap.Subscription)
typeDefinitionsMap.Subscription = [EmptySubscriptionDefinition];

return {
typeToServiceMap,
Expand Down