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

Code-First Federation 2 Support #3921

Open
wants to merge 127 commits into
base: addfederation_extension
Choose a base branch
from
Open

Conversation

Shane32
Copy link
Member

@Shane32 Shane32 commented Feb 26, 2024

Based on #3144 by @Mithras

  • New code is in namespace GraphQL.Federation
  • Old code in namespace GraphQL.Utilities.Federation has been marked as obsolete
  • Federation code is unified into .AddFederation() for both schema-first, code-first and type-first use cases.
  • Old schema-first syntax is marked as obsolete, but will function without any (or very few) changes
  • Code first supported, mostly by @Mithras
  • Type first supported
  • Apollo Router now runs an actual test against four live subgraph schemas (aka demo apps) to form a single supergraph:
    • Schema first, new syntax, federation 1 compatible
    • Schema first, old syntax, federation 1 compatible
    • Code first, federation 2 compatible
    • Type first, federation 2 compatible

Breaking changes:

  • A couple classes were moved.
  • The __typename field is not implicitly added to federation requests

Prerequisites:

Split into separate PRs:

To-do / nice-to-have:

  • Analyzer rule for new [FederationResolver] attribute to ensure it is used correctly
  • Simplify defining federation keys #3962
  • New tests for Sample3 and Sample4 web applications
  • Additional federation testing for Sample3 and Sample4
  • Move new extension methods to GraphQL.Federation namespace rather than GraphQL.Federation.Extensions
  • Add documentation
  • Break PR into smaller pieces and merge each piece separately
  • Merge master/develop into PR
  • Add xml comments to all federation classes/etc

Moved out of scope:

@Shane32 Shane32 self-assigned this Feb 26, 2024
src/GraphQL.sln Outdated Show resolved Hide resolved
src/GraphQL.sln Outdated Show resolved Hide resolved
Comment on lines 39 to 42
if (SourceType == typeof(object) || SourceType == typeof(Dictionary<string, object?>) || SourceType == typeof(IDictionary<string, object?>))
return representation;
else
return ToObject(SourceType, graphType, representation);

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note

Both branches of this 'if' statement return - consider using '?' to express intent better.
@Shane32 Shane32 changed the base branch from develop to federation_resolver_attribute June 18, 2024 00:11
Base automatically changed from federation_resolver_attribute to develop June 20, 2024 13:41
settings.SdlPrintOptions = new() { IncludeFederationTypes = false };

// todo: ensure all directives are supported by all supported versions
var directives = settings.ImportDirectives ?? FederationDirectiveEnum.All;

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
directives
is useless, since its value is never read.
@Shane32 Shane32 changed the base branch from develop to addfederation_extension June 20, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
federation Relates to GraphQL Federation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants