Support for @defer #1108
Support for @defer #1108
Conversation
WalkthroughThis change introduces comprehensive support for GraphQL Changes
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
.gitignore (1)
8-8:*vendor*pattern is overly broadUsing leading and trailing wildcards will ignore any path containing the substring “vendor”, e.g.
frontend/avendorize.cssormy_vendor_plugin.go.
If the intent is to exclude only top-level Go or JS vendor folders, a tighter pattern such asvendor/**or/vendor/is safer.-*vendor* +/vendor/**v2/pkg/engine/resolve/node_object.go (1)
187-190: Address TODO: Add support for @defer directive arguments.The
@deferdirective supportslabelandifarguments according to the GraphQL spec. These should be added to fully support the defer directive functionality.Would you like me to help implement the Label and If fields and their handling logic, or create an issue to track this enhancement?
v2/pkg/engine/plan/defer_visitor.go (1)
79-81: Remove commented duplicate code.Line 79 appears to be a duplicate of line 80. If this was intentional for debugging, it should be removed.
- // v.deferredFragmentStack = append(v.deferredFragmentStack, info) v.deferredFragments = append(v.deferredFragments, info) v.deferredFragmentStack = append(v.deferredFragmentStack, info)v2/pkg/engine/resolve/resolve.go (1)
260-303: Consider adding timeout protection for recursive deferred resolution.The recursive resolution of deferred responses could potentially run indefinitely if there are many nested defers. Consider adding a maximum depth or timeout protection.
-func (r *Resolver) doResolve(ctx *Context, t *tools, response *GraphQLResponse, data []byte, writer io.Writer, isFinal bool) error { +func (r *Resolver) doResolve(ctx *Context, t *tools, response *GraphQLResponse, data []byte, writer io.Writer, isFinal bool) error { + // Consider adding a deadline to prevent runaway deferred resolution + if deadline, ok := ctx.ctx.Deadline(); ok && time.Until(deadline) < time.Second { + return fmt.Errorf("insufficient time remaining for deferred resolution") + }v2/pkg/engine/plan/planner.go (1)
235-248: Correct implementation of defer-aware planning paths.The logic properly creates separate planning paths for the main operation and each deferred fragment. The planner reset on line 372 correctly isolates results per defer target.
Consider pre-allocating the planners slice if performance becomes a concern with many deferred fragments:
- planners := p.configurationVisitor.planners + planners := make([]PlannerConfiguration, 0, len(p.configurationVisitor.planners)*(1+len(p.deferVisitor.deferredFragments))) + planners = append(planners, p.configurationVisitor.planners...)Also applies to: 371-372
v2/pkg/engine/resolve/multipart_json_writer.go (2)
10-11: Consider finalizing the package location.The TODO comment suggests uncertainty about where this code should reside. Consider moving it to a dedicated package if multipart response handling grows in scope.
58-71: Simplify redundant path assignment.The path is set twice - unconditionally at line 62 and conditionally at lines 68-69. The conditional check is redundant.
if w.wroteInitial { part.Incremental = []incrementalDataPart{ { Data: part.Data, Path: path, }, } part.Data = nil - - if len(path) > 0 { - part.Incremental[0].Path = path - } }v2/pkg/engine/postprocess/extract_deferred_fields.go (1)
79-104: Complex but correct field placement logic.The logic correctly distributes fields between immediate and deferred responses. The TODO comment at line 84 suggests awareness of potential improvements.
Consider extracting the field placement logic into a helper method for better readability:
func (v *deferredFieldsVisitor) shouldAddToResponse(field *resolve.Field, resp *responseItem) bool { // Extract the complex logic here }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.gitignore(1 hunks)v2/pkg/ast/path.go(2 hunks)v2/pkg/ast/path_test.go(1 hunks)v2/pkg/asttransform/baseschema.go(1 hunks)v2/pkg/engine/plan/analyze_plan_kind.go(1 hunks)v2/pkg/engine/plan/analyze_plan_kind_test.go(3 hunks)v2/pkg/engine/plan/configuration_visitor.go(5 hunks)v2/pkg/engine/plan/defer_visitor.go(1 hunks)v2/pkg/engine/plan/plan.go(1 hunks)v2/pkg/engine/plan/planner.go(8 hunks)v2/pkg/engine/plan/planner_test.go(3 hunks)v2/pkg/engine/plan/visitor.go(6 hunks)v2/pkg/engine/postprocess/extract_deferred_fields.go(1 hunks)v2/pkg/engine/postprocess/extract_deferred_fields_test.go(1 hunks)v2/pkg/engine/postprocess/plan_visitor.go(3 hunks)v2/pkg/engine/postprocess/postprocess.go(4 hunks)v2/pkg/engine/resolve/fetch.go(2 hunks)v2/pkg/engine/resolve/multipart_json_writer.go(1 hunks)v2/pkg/engine/resolve/multipart_response_writer_test.go(1 hunks)v2/pkg/engine/resolve/node_object.go(4 hunks)v2/pkg/engine/resolve/node_object_test.go(1 hunks)v2/pkg/engine/resolve/resolve.go(1 hunks)v2/pkg/engine/resolve/resolve_test.go(2 hunks)v2/pkg/engine/resolve/response.go(2 hunks)
🧰 Additional context used
🧠 Learnings (9)
v2/pkg/engine/resolve/multipart_response_writer_test.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/resolve/resolve_test.go (1)
undefined
<retrieved_learning>
Learnt from: SkArchon
PR: #1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
</retrieved_learning>
v2/pkg/engine/resolve/response.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/resolve/node_object.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/postprocess/extract_deferred_fields_test.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/plan/planner_test.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/resolve/resolve.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/resolve/multipart_json_writer.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
v2/pkg/engine/plan/visitor.go (1)
Learnt from: SkArchon
PR: wundergraph/graphql-go-tools#1203
File: v2/pkg/engine/resolve/loader.go:63-67
Timestamp: 2025-07-02T15:28:02.122Z
Learning: In the graphql-go-tools codebase, result structs are consistently initialized with non-nil bytes.Buffer instances, making additional nil checks for res.out unnecessary defensive programming.
🧬 Code Graph Analysis (12)
v2/pkg/engine/plan/analyze_plan_kind.go (1)
pkg/ast/ast_node_kind.go (2)
NodeKindInlineFragment(35-35)NodeKindFragmentSpread(34-34)
v2/pkg/engine/plan/plan.go (2)
v2/pkg/engine/resolve/response.go (1)
GraphQLResponse(24-31)v2/pkg/engine/resolve/node_object.go (1)
DeferInfo(187-190)
v2/pkg/ast/path_test.go (1)
v2/pkg/ast/path.go (4)
Path(34-34)FieldName(23-23)ArrayIndex(22-22)InlineFragmentName(24-24)
v2/pkg/engine/plan/defer_visitor.go (3)
v2/pkg/engine/resolve/node_object.go (1)
DeferInfo(187-190)v2/pkg/lexer/literal/literal.go (1)
DEFER(68-68)v2/pkg/ast/path.go (1)
InlineFragmentName(24-24)
v2/pkg/engine/postprocess/postprocess.go (2)
v2/pkg/engine/resolve/response.go (1)
GraphQLResponse(24-31)pkg/engine/resolve/resolve.go (1)
Fetches(318-318)
v2/pkg/engine/resolve/response.go (3)
v2/pkg/engine/resolve/node_object.go (1)
Object(10-20)v2/pkg/engine/resolve/fetchtree.go (1)
FetchTreeNode(8-15)v2/pkg/engine/resolve/loader.go (1)
DataSourceInfo(44-47)
v2/pkg/engine/resolve/node_object.go (1)
v2/pkg/ast/path.go (4)
Path(34-34)InlineFragmentName(24-24)ArrayIndex(22-22)FieldName(23-23)
v2/pkg/engine/postprocess/extract_deferred_fields_test.go (4)
v2/pkg/engine/resolve/node_object.go (3)
DeferInfo(187-190)Object(10-20)Field(94-103)v2/pkg/engine/resolve/response.go (1)
GraphQLResponseInfo(33-35)v2/pkg/engine/resolve/fetch.go (1)
FetchDependencies(99-102)v2/pkg/ast/path.go (1)
InlineFragmentName(24-24)
v2/pkg/engine/resolve/fetch.go (1)
v2/pkg/engine/resolve/node_object.go (1)
DeferInfo(187-190)
v2/pkg/engine/resolve/resolve.go (2)
v2/pkg/engine/resolve/response.go (2)
IncrementalResponseWriter(51-55)GraphQLResponse(24-31)v2/pkg/engine/resolve/context.go (2)
Context(16-34)ExecutionOptions(36-40)
v2/pkg/engine/plan/planner.go (5)
v2/pkg/engine/plan/configuration.go (1)
Configuration(9-30)v2/pkg/astvisitor/visitor.go (1)
Walker(65-95)pkg/astvisitor/visitor.go (1)
Walker(19-49)v2/pkg/engine/resolve/node_object.go (1)
DeferInfo(187-190)pkg/operationreport/operationreport.go (1)
Report(9-12)
v2/pkg/engine/resolve/multipart_json_writer.go (2)
v2/pkg/engine/resolve/response.go (1)
IncrementalResponseWriter(51-55)v2/pkg/graphqlerrors/errors.go (1)
Extensions(141-143)
🔇 Additional comments (40)
.gitignore (1)
2-2: Good call adding.vscode/to the ignore listThis keeps local VS Code settings out of the repo.
v2/pkg/asttransform/baseschema.go (1)
159-174: LGTM! Well-structured directive definitions.The
@deferand@streamdirective definitions are correctly implemented with proper:
- Documentation strings explaining their purpose
- Argument definitions with appropriate defaults (
if: Boolean! = true)- Location restrictions that align with GraphQL specification
- Placement in the base schema between existing directives
The syntax and semantics match the GraphQL incremental delivery specification.
v2/pkg/engine/plan/plan.go (1)
21-22: Good addition for defer tracking.The new fields appropriately track deferred execution metadata:
DeferredFragments []resolve.DeferInfo- for deferred fragment spreads/inline fragmentsDeferredFields map[int]resolve.DeferInfo- for deferred fields with field reference as keyThe data structures align with expected access patterns and integrate well with the resolve package's DeferInfo type.
v2/pkg/engine/plan/analyze_plan_kind_test.go (3)
103-105: Correct usage pattern for @defer directive.The change from
favoriteEpisode @deferto... @defer { favoriteEpisode }aligns with the directive locations defined in the base schema, where@deferis only allowed onFRAGMENT_SPREADandINLINE_FRAGMENTlocations, not directly on fields.
151-153: Consistent with directive location constraints.The inline fragment syntax correctly implements the @defer directive usage pattern.
174-176: Proper @defer usage in subscription test.The test correctly uses inline fragment syntax for the @defer directive in subscription context.
v2/pkg/engine/plan/analyze_plan_kind.go (1)
47-56: Correct defer directive handling for fragment locations.The implementation properly handles
@deferdirectives onInlineFragmentandFragmentSpreadnodes, aligning with the directive locations defined in the base schema. The logic correctly:
- Detects defer directives on inline fragments and fragment spreads
- Maintains stream directive detection only on fields
- Removes defer handling from field nodes (which was incorrect)
This ensures proper plan kind analysis for deferred execution.
v2/pkg/ast/path_test.go (1)
10-106: Comprehensive test coverage for path overlap logic.The test suite provides excellent coverage for the
Overlapsmethod with well-designed test cases covering:
- Edge cases (empty paths, single elements)
- Different PathItem kinds (FieldName, ArrayIndex, InlineFragmentName)
- Prefix matching scenarios
- Mixed path combinations
- Symmetry verification (
a.Overlaps(b) == b.Overlaps(a))This testing is crucial for the defer directive implementation, as path overlap detection is fundamental to determining deferred execution boundaries.
v2/pkg/engine/resolve/fetch.go (2)
83-83: Field addition looks correct for @defer directive support.The
DeferInfo *DeferInfofield addition to theSingleFetchstruct aligns with the broader @defer directive implementation. This field will carry defer metadata as mentioned in the AI summary.
58-60: DeferInfo.Equals null‐safety confirmedThe
Equalsmethod is defined in v2/pkg/engine/resolve/node_object.go (lines 192–197) and first checksif d == nil || other == nil { return d == other }, so it safely handles all nil combinations without panic. No further action needed.v2/pkg/engine/resolve/resolve_test.go (2)
11-11: LGTM: Import addition is appropriate.The
stringspackage import is correctly added and used for formatting the expected multipart response output in the new test.
4773-4892: Excellent test implementation for incremental GraphQL responses.This test comprehensively validates the new
@deferfunctionality with several strengths:
- Comprehensive test data: The test includes both initial data and deferred responses with proper field configurations and type constraints
- Multipart handling: Correctly uses
MultipartJSONWriterto simulate multipart HTTP responses with proper boundary tokens- Expected output validation: Properly formats expected multipart response with CRLF line endings using
strings.ReplaceAll- Follows established patterns: Consistent with other test functions in the file using table-driven approach
The test effectively validates the core functionality introduced in this PR for GraphQL incremental responses.
v2/pkg/engine/resolve/response.go (2)
25-30: LGTM! Well-structured support for deferred responses.The addition of
DeferredResponsesfield creates an appropriate recursive structure for handling nested deferred GraphQL responses. Using a slice of pointers allows for optional deferred responses without memory overhead when not used.
51-55: LGTM! Clean interface design for incremental response handling.The
IncrementalResponseWriterinterface appropriately extendsResponseWriterwith methods needed for streaming deferred responses. TheFlushmethod's parameters (pathandisFinal) provide the necessary context for incremental delivery.v2/pkg/engine/plan/planner_test.go (1)
307-753: Comprehensive test coverage for defer planning!The test suite thoroughly validates defer directive handling with:
- Proper DeferPaths on fields within deferred inline fragments
- Correct DeferInfo on fetch operations
- Accurate tracking in DeferredFragments and DeferredFields maps
- Both simple and array-based scenarios
The test structure and assertions properly verify the planner's defer functionality.
v2/pkg/engine/postprocess/postprocess.go (3)
28-28: LGTM! Proper initialization of extractDeferredFields processor.The field is correctly added to the Processor struct and initialized in the constructor.
Also applies to: 135-135
145-148: LGTM! Clean separation of deferred fragment processing.The conditional processing of deferred fragments is appropriately placed, and the refactoring to use
processFetchTreesimproves code organization.
198-208: Excellent recursive processing of deferred fetch trees!The method properly consolidates fetch tree processing and ensures deferred responses receive the same processing pipeline as the main response through recursive calls.
v2/pkg/ast/path.go (2)
57-73: LGTM! Correct implementation of path overlap checking.The method properly checks if one path is a prefix of another by comparing all relevant fields (Kind, FragmentRef, ArrayIndex, FieldName) and correctly returns true when the first path is exhausted.
98-127: Clean refactoring of string conversion methods!The separation of concerns between
Path.StringSlice()andPathItem.String()improves code organization. The string representations for each PathKind are appropriate and consistent.v2/pkg/engine/postprocess/plan_visitor.go (2)
72-82: LGTM! Appropriate visibility change for visitor registration.Making the registration methods public enables external components like
extractDeferredFieldsto register as visitors, which is necessary for the deferred field extraction functionality.
157-158: Good consistency improvements.Moving the path operations and using wrapper methods for visitor calls improves code consistency throughout the walker implementation.
Also applies to: 183-184
v2/pkg/engine/resolve/node_object.go (2)
98-98: LGTM! Appropriate refactoring for multi-defer support.The change from a single defer pointer to a slice of defer paths enables proper support for multiple deferred fragments, which aligns with the GraphQL spec allowing multiple defer directives.
Also applies to: 115-115
206-242: Array index wildcard and inline fragment skipping are fully covered by existing tests
- The
HasPrefixmethod treats"@"as the array‐index wildcard by design, and this behavior is exercised in the"handle arrays"and"handle arrays, but mis-match"test cases innode_object_test.go.- The skip logic for inline fragments is validated by the
"ignore inline fragment"and"ignore terminal inline fragment"tests.No further changes are required.
v2/pkg/engine/postprocess/extract_deferred_fields_test.go (1)
15-788: Well-structured and comprehensive test coverage!The tests thoroughly cover:
- Trivial case with no deferred fields
- Simple deferred fields with inline fragments
- Complex array scenarios with deferred fields
The test structure properly validates that deferred fields are extracted into separate responses while preserving the main response structure.
v2/pkg/engine/resolve/multipart_response_writer_test.go (2)
16-89: LGTM!Comprehensive test coverage for multipart JSON response generation, including proper boundary handling, content types, and incremental data structures.
91-213: LGTM!Well-structured unit tests with comprehensive error case coverage and proper test helper implementation.
v2/pkg/engine/plan/planner.go (1)
71-80: LGTM!Clean integration of the defer visitor following the established walker pattern. The visitor properly collects defer metadata during node selection.
Also applies to: 177-178
v2/pkg/engine/resolve/multipart_json_writer.go (1)
72-124: LGTM!The JSON encoding, boundary handling, and type definitions are well-implemented. The trailing newline removal ensures proper multipart formatting.
v2/pkg/engine/plan/configuration_visitor.go (3)
371-378: LGTM!Clean implementation of fragment visitor methods with proper delegation to the common defer handling logic.
389-406: LGTM!The defer directive removal and fragment skipping logic is correctly implemented. The AST modification properly removes the directive to prevent downstream processing.
379-387: LGTM!Proper implementation of defer-aware field filtering using path overlap checking. The defer info is correctly propagated to the fetch configuration.
Also applies to: 415-419, 884-884
v2/pkg/engine/postprocess/extract_deferred_fields.go (3)
13-53: LGTM!Well-structured initialization and orchestration. The separation of immediate and deferred response items is clean and the walker pattern is properly applied.
62-75: LGTM!Correct object stack management for traversing nested structures. The implementation properly handles both Object and Array types.
Also applies to: 106-123, 145-164
125-217: LGTM!Well-implemented helper methods with proper deep copying and filtering logic. Good use of Go 1.23 iterators for clean iteration over matching response items.
v2/pkg/engine/plan/visitor.go (5)
50-51: LGTM!The addition of
deferredFragmentsanddeferredFieldsfields properly extends the Visitor struct to track defer metadata.
339-351: LGTM!The defer path handling correctly associates deferred fragments and fields with the current field, while preventing duplicate paths.
371-382: LGTM!The
followingDeferredFragmentPathsmethod correctly implements an iterator to find deferred fragments below the current field path. The use ofWithoutInlineFragmentNames()ensures accurate path matching.
912-914: LGTM!The response plan initialization correctly includes the collected defer metadata.
1211-1211: LGTM!The assignment of
DeferInfocorrectly propagates defer metadata to the fetch configuration.
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| assert.Equal(t, tt.expected, tt.input.Overlaps(tt.otherPath)) | ||
| // TODO: reflexive property test. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complete the reflexive property test for Overlaps.
The TODO indicates missing test for the reflexive property of path overlapping. This is important to ensure the Overlaps method is symmetric.
Add the reflexive test after line 153:
assert.Equal(t, tt.expected, tt.input.Overlaps(tt.otherPath))
-// TODO: reflexive property test.
+// Test reflexive property: if A overlaps B, then B should overlap A
+if tt.input != nil && len(tt.otherPath) > 0 {
+ otherDeferInfo := &DeferInfo{Path: tt.otherPath}
+ assert.Equal(t, tt.expected, otherDeferInfo.Overlaps(tt.input.Path),
+ "reflexive property failed: Overlaps should be symmetric")
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: reflexive property test. | |
| assert.Equal(t, tt.expected, tt.input.Overlaps(tt.otherPath)) | |
| // Test reflexive property: if A overlaps B, then B should overlap A | |
| if tt.input != nil && len(tt.otherPath) > 0 { | |
| otherDeferInfo := &DeferInfo{Path: tt.otherPath} | |
| assert.Equal(t, tt.expected, otherDeferInfo.Overlaps(tt.input.Path), | |
| "reflexive property failed: Overlaps should be symmetric") | |
| } |
🤖 Prompt for AI Agents
In v2/pkg/engine/resolve/node_object_test.go at line 154, the reflexive property
test for the Overlaps method is missing. Add a test case that verifies for a
given path, Overlaps returns true when compared with itself, ensuring the method
is symmetric. Insert this test immediately after line 153 to complete the
coverage.
| func (v *deferVisitor) EnterFragmentSpread(ref int) { | ||
| // TODO(cd): Fragment spreads are expanded to inline fragments during normalization. Skipping these. | ||
| } | ||
|
|
||
| func (v *deferVisitor) LeaveFragmentSpread(ref int) { | ||
| } |
There was a problem hiding this comment.
Address TODO: Implement fragment spread handling.
Fragment spreads with @defer directive need to be handled according to the GraphQL spec. The current implementation skips them entirely.
Would you like me to help implement proper fragment spread handling for deferred execution, or should we create an issue to track this limitation?
🤖 Prompt for AI Agents
In v2/pkg/engine/plan/defer_visitor.go around lines 54 to 59, the
EnterFragmentSpread method currently skips handling fragment spreads with a TODO
comment. To fix this, implement logic to properly process fragment spreads that
have the @defer directive according to the GraphQL specification, ensuring
deferred execution is handled correctly. This may involve expanding fragment
spreads to inline fragments or appropriately marking them for deferred
processing instead of skipping them.
| func resolvedPath(data []string) []any { | ||
| if len(data) == 0 { | ||
| return nil | ||
| } | ||
| ret := make([]any, len(data)) | ||
| for i, v := range data { | ||
| if v == "@" { | ||
| ret[i] = 0 // TODO(cd): need the real values here. | ||
| continue | ||
| } | ||
| ret[i] = v | ||
| } | ||
| return ret | ||
| } |
There was a problem hiding this comment.
Fix TODO: Implement proper array index resolution.
The resolvedPath function currently uses 0 as a placeholder for array indices (marked with "@"), but this needs the actual array index values for correct path resolution in incremental responses.
This could lead to incorrect path information in deferred responses when arrays are involved. The actual array indices should be tracked and propagated through the resolution process.
if v == "@" {
- ret[i] = 0 // TODO(cd): need the real values here.
+ // This requires tracking actual array indices during resolution
+ // For now, this will cause issues with array path resolution
+ ret[i] = 0
continue
}Would you like me to help implement proper array index tracking throughout the resolution process?
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In v2/pkg/engine/resolve/resolve.go around lines 305 to 318, the resolvedPath
function currently replaces "@" with 0 as a placeholder for array indices, which
is incorrect. To fix this, modify the function to track and insert the actual
array index values instead of 0. This requires passing the current array index
context into the function or accessing it from the resolution process, ensuring
that each "@" is replaced with the correct index to maintain accurate path
resolution in incremental responses.
| @@ -3,6 +3,7 @@ package plan | |||
| import ( | |||
| "bytes" | |||
| "fmt" | |||
| "iter" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Go version compatibility for iter package.
The iter package requires Go 1.22 or later. Please ensure the project's minimum Go version requirement is compatible with this import.
🏁 Script executed:
#!/bin/bash
# Description: Check the project's Go version requirement from go.mod
cat go.mod | grep -E "^go\s+"Length of output: 39
Upgrade minimum Go version or replace iter usage
The iter package, introduced in Go 1.22, is not supported under the project’s current Go 1.20 setting. Please choose one of the following:
• Bump the Go version in go.mod to 1.22 (and update CI/tooling accordingly)
• Remove the iter import and implement an alternative iterator compatible with Go 1.20
Affected file:
- go.mod (currently
go 1.20)
🤖 Prompt for AI Agents
In v2/pkg/engine/plan/visitor.go at line 6, the import of the `iter` package
requires Go 1.22, but the project currently uses Go 1.20. To fix this, either
update the Go version in go.mod to 1.22 and adjust CI and tooling to support it,
or remove the `iter` import and replace its usage with a custom iterator
implementation compatible with Go 1.20.
Works on simple cases, but takes a naive approach, fetching the whole path rather than reusing previously-fetched data.
Example query:
Some things still needed:
__typenamein fragment selection setsSummary by CodeRabbit
New Features
@deferand@streamdirectives, enabling incremental and deferred delivery of query results.@deferand@streamdirectives.Bug Fixes
Tests
Documentation
Chores
.gitignoreto exclude VS Code workspace files.