Fix @join__field emission for Fed v1 @external and @override+ @requires edge cases#284
Fix @join__field emission for Fed v1 @external and @override+ @requires edge cases#284n1ru4l merged 2 commits intokamilkisiela-patch-1from
@join__field emission for Fed v1 @external and @override+ @requires edge cases#284Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors and enhances the supergraph composition process, specifically addressing how Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and valuable refactoring of the @override composition logic, centralizing it to improve maintainability and fix complex bugs related to interactions with @requires and interfaces. The addition of supergraph validation using @apollo/federation-internals in the testkit is an excellent enhancement for ensuring correctness. The code is much cleaner and more consistent now. I have one suggestion to further improve the new logic by reducing some code duplication.
@join__field emission for Fed v1 @external and @override+ @requires edge cases
…@requires` edge cases (#284) This PR corrects supergraph's `@join__field` emission in a few edge cases where emitted metadata could be incorrect or redundant. - Use subgraph-local key usage when deciding whether to drop Federation v1 `@join__field(external:true)` - Avoid emitting redundant `@join__field(external: true)` metadata when a field is only required through overridden paths - Tighten `@join__field` emission around `@override` and interface type fields. This aligns emitted supergraph metadata with Apollo Gateway's requirements. Added Apollo supergraph validation in testkit to catch compatibility regressions earlier.
This PR corrects supergraph's
@join__fieldemission in a few edge cases where emitted metadata could be incorrect or redundant.@join__field(external:true)@join__field(external: true)metadata when a field is only required through overridden paths@join__fieldemission around@overrideand interface type fields.This aligns emitted supergraph metadata with Apollo Gateway's requirements.
Added Apollo supergraph validation in testkit to catch compatibility regressions earlier.