Fix @join__field emission for Fed v1 @external and @override+ @requires edge cases#282
Fix @join__field emission for Fed v1 @external and @override+ @requires edge cases#282kamilkisiela merged 5 commits intomainfrom
@join__field emission for Fed v1 @external and @override+ @requires edge cases#282Conversation
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 expands the test suite for Apollo Federation v2 composition, focusing on the accurate generation of @join__field directives. It specifically addresses scenarios involving object type extensions where @key fields are marked as @external, and when @external fields are further utilized with the @requires directive. These additions bolster the reliability of supergraph schema composition under advanced federation patterns. Highlights
ChangelogActivity
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 two new test cases to composition.spec.ts to enhance test coverage for @join__field directive composition, specifically for object type extensions. The tests cover scenarios where key fields are marked as @external and where other external fields are utilized in @requires directives. The added tests are clear, target specific edge cases, and the assertions appear correct based on the expected composition behavior. The changes are a good addition to the test suite.
…@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.
2103359 to
de2d7db
Compare
@join__field emission for Fed v1 @external and @override+ @requires edge cases
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @theguild/federation-composition@0.22.1 ### Patch Changes - [#291](#291) [`ec0ef84`](ec0ef84) Thanks [@jdolle](https://github.com/jdolle)! - Support "file:" protocol and non-RFC 3986 in imported "link" url argument - [#282](#282) [`e507fdd`](e507fdd) Thanks [@kamilkisiela](https://github.com/kamilkisiela)! - Fix supergraph `@join__field` emission for Federation v1 `@external` fields and improve override/requires edge-case handling. - Drop Federation v1 `@join__field(external: true)` fields based on key usage in the subgraph instead of aggregated field key usage across subgraphs. - 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. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Reproduced issues caused by #267 and added a few test cases to showcase differences between Apollo's supergraph and ours.