feat(composition): support @provides on fields returning a Union type#3026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThis PR adds union support to ChangesProvides validation update
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3026 +/- ##
===========================================
- Coverage 61.44% 43.34% -18.10%
===========================================
Files 259 867 +608
Lines 30137 125298 +95161
Branches 0 10192 +10192
===========================================
+ Hits 18517 54312 +35795
- Misses 10131 70640 +60509
+ Partials 1489 346 -1143
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composition/src/v1/normalization/normalization-factory.ts (1)
2061-2122: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftValidate inline fragments by possible-type overlap
composition/src/v1/normalization/normalization-factory.ts:2061-2122The kind-based checks here miss GraphQL’s overlap rule: interface fragments on a union parent are rejected even when they share concrete members, and union type conditions are accepted unconditionally even when they are disjoint. Replace the shortcut logic with a possible-type intersection check for the parent and fragment type condition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composition/src/v1/normalization/normalization-factory.ts` around lines 2061 - 2122, The inline fragment validation in normalization-factory is using kind-specific shortcuts instead of GraphQL possible-type overlap, so update the logic around the fragmentNamedTypeData switch to compare the parent and typeCondition by intersecting their possible concrete types. In the fragment handling path near the existing parentDatas/shouldDefineSelectionSet flow, keep the existing error helpers but replace the interface/union/object acceptance rules with a shared overlap check for parentData and fragmentNamedTypeData, and only allow the fragment when their possible types intersect; otherwise fall through to invalidInlineFragmentTypeConditionErrorMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@composition/src/errors/errors.ts`:
- Around line 779-781: Update the incompatible `@provides` error message in the
error helper that builds the response-type validation error so it includes Union
as a valid kind alongside Object and Interface, matching the new
VALID_PROVIDES_PARENT_KINDS behavior. While touching that return in errors.ts,
collapse the message construction into a single template literal instead of
concatenating strings, keeping the same fieldCoords, subgraphName, and
responseType placeholders.
In `@composition/tests/v1/directives/provides.test.ts`:
- Around line 1242-1268: The union-returning `@provides` tests only verify that
warnings are empty, so they do not prove the directive is preserved in the
composed result. Update the relevant tests around the
createSubgraph/federateSubgraphsSuccess flow to assert the composed output for
the union parent path, especially subgraphConfigBySubgraphName and the
conditional-field data produced for the `@provides` selection. Keep the existing
no-warning check, but add assertions that the composed config still contains the
union-field `@provides` behavior for the valid union cases.
---
Outside diff comments:
In `@composition/src/v1/normalization/normalization-factory.ts`:
- Around line 2061-2122: The inline fragment validation in normalization-factory
is using kind-specific shortcuts instead of GraphQL possible-type overlap, so
update the logic around the fragmentNamedTypeData switch to compare the parent
and typeCondition by intersecting their possible concrete types. In the fragment
handling path near the existing parentDatas/shouldDefineSelectionSet flow, keep
the existing error helpers but replace the interface/union/object acceptance
rules with a shared overlap check for parentData and fragmentNamedTypeData, and
only allow the fragment when their possible types intersect; otherwise fall
through to invalidInlineFragmentTypeConditionErrorMessage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 73b11a7c-85b6-4368-8689-78f7025b2741
📒 Files selected for processing (8)
composition/src/ast/utils.tscomposition/src/errors/errors.tscomposition/src/schema-building/types/types.tscomposition/src/utils/string-constants.tscomposition/src/v1/normalization/normalization-factory.tscomposition/src/v1/normalization/types/results.tscomposition/src/v1/normalization/types/types.tscomposition/tests/v1/directives/provides.test.ts
comatory
approved these changes
Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
@providesvalidation by returningErrorobjects with more precise messaging for incompatible types and invalid inline-fragment type conditions.@providesparent type scenarios, including union-returning fields, while still enforcing incompatible base-scalar cases.@provideson union-returning fields when router support may be limited.@provides, including invalid@external, unknown/invalid inline fragment conditions, and non-member union fragment cases.Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.