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

fix(deps) Upgrade apollo federation with subgraph #1797

Merged
merged 4 commits into from
Oct 20, 2021
Merged

fix(deps) Upgrade apollo federation with subgraph #1797

merged 4 commits into from
Oct 20, 2021

Conversation

iMobs
Copy link
Contributor

@iMobs iMobs commented Oct 16, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #1782

What is the new behavior?

Directives and printSchema (now renamed printSubgraphSchema) are imported from the @apollo/subgraph module instead of from @apollo/federation.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Recently the @apollo/subgraph module was created and split off from the @apollo/federation module (PR) and released that as a patch update. This broke using @nestjs/graphql with the newest version of @apollo/federation. The most obvious problem was with the optional import of directives reported in #1782.

I believe this overlaps and resolves the same issue as #1780 since this fixes the federation use of printSchema without changing the normal use of printSchema.

Closes #1782

@iMobs
Copy link
Contributor Author

iMobs commented Oct 17, 2021

Looking again at where @apollo/federation is used, it could be replaced entirely with @apollo/subgraph since the only thing still in use is buildFederatedSchema which is deprecated and a re-export of buildSubgraphSchema. This seems like a more drastic step so I'll wait for initial feedback.

Ian Mobley added 4 commits October 17, 2021 12:32
Recently the subgraph package was split off from the main federation
package. This moved the federation directives and renamed printSchema
to printSubgraphSchema.
This is now part of the subgraph dependency.
This is now part of the subgraph dependency and renamed to
printSubgraphSchema.
This is now part of the subgraph dependency and the type defs do not
need to be duplicated.
@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Oct 18, 2021

Looking again at where @apollo/federation is used, it could be replaced entirely with @apollo/subgraph since the only thing still in use is buildFederatedSchema which is deprecated and a re-export of buildSubgraphSchema. This seems like a more drastic step so I'll wait for initial feedback.

Agree. Would you like to create a follow-up PR that entirely replaces the @apollo/federation package? We'll surely merge it (eventually!)

I'll merge this one shortly, thanks!

@ssukienn
Copy link

@iMobs Just for reference this or follow-up PRs are not resolving #1780 case because in the place of "schema to file generation" (GraphQLSchemaBuilder.buildSchema) there is still used printSchema of graphql package instead of apollo federation or subgraph. Compare the changes and you will see that they do not overlap (not completely at least).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An issue with the Apollo Federation dependency.
3 participants