-
Notifications
You must be signed in to change notification settings - Fork 158
feat: add field argument mapping #1373
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
Closed
dkorittki
wants to merge
27
commits into
master
from
dominik/eng-8826-add-field-argument-mapping-support-to-engine
Closed
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
ce23ea5
feat: store field arg maps to variable normalization
dkorittki 1ebb2c3
chore: reduce two functions to one
dkorittki ea5c630
fix: handle fragments
dkorittki 73a4ac5
fix: support literal values
dkorittki cd47b12
chore: move mapping to operation normalizer
dkorittki 1b8bddc
Revert "chore: move mapping to operation normalizer"
dkorittki fdf63d0
fix: expose literal values as astjson object
dkorittki a85a37b
chore: remove fragment support
dkorittki 546a496
feat: make field arg mapping configurable
dkorittki e2c713b
Merge branch 'master'
dkorittki 75236fc
Merge branch 'master'
dkorittki e4c44d3
fix: record mapping for already extracted literals
dkorittki f629d8d
fix: use valid operation for test
dkorittki 9877606
fix: support inline fragments
dkorittki 94178d4
Merge branch 'master'
dkorittki 15dbb3f
fix: add missing argument after main merge
dkorittki f7a1c98
chore: preserve original method signature
dkorittki 40a7750
chore: add path string creation tests
dkorittki 0fbb313
chore: add godoc to variable normalizer methods
dkorittki 1166211
chore: use string concat on path build instead of Sprintf
dkorittki 1e8d2a5
Merge branch 'master' into dominik/eng-8826-add-field-argument-mappin…
dkorittki 7d265d7
chore: update godoc/comments + result type name
dkorittki a43dae9
chore: seperate file for VariablesNormalizer
dkorittki 2462573
chore: Add options type for VariablesNormalizer
dkorittki 0d27f89
fix: fix broken tests
dkorittki 55c1a82
Merge branch 'master' into dominik/eng-8826-add-field-argument-mappin…
dkorittki f4e3312
Merge branch 'master' into dominik/eng-8826-add-field-argument-mappin…
dkorittki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,7 +70,6 @@ package astnormalization | |
|
|
||
| import ( | ||
| "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" | ||
| "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" | ||
| "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" | ||
| "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" | ||
| ) | ||
|
|
@@ -245,7 +244,8 @@ func (o *OperationNormalizer) setupOperationWalkers() { | |
|
|
||
| if o.options.extractVariables { | ||
| extractVariablesWalker := astvisitor.NewWalkerWithID(8, "ExtractVariables") | ||
| extractVariables(&extractVariablesWalker) | ||
| // disabling field arg mapping as it's only necessary on the variable normalizer | ||
| extractVariables(&extractVariablesWalker, false) | ||
| o.operationWalkers = append(o.operationWalkers, walkerStage{ | ||
| name: "extractVariables", | ||
| walker: &extractVariablesWalker, | ||
|
|
@@ -353,7 +353,7 @@ type VariablesNormalizer struct { | |
| variablesExtractionVisitor *variablesExtractionVisitor | ||
| } | ||
|
|
||
| func NewVariablesNormalizer() *VariablesNormalizer { | ||
| func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { | ||
|
Noroth marked this conversation as resolved.
Outdated
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about introducing either an options or config argument? e.g. or just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
| // delete unused modifying variables refs, | ||
| // so it is safer to run it sequentially with the extraction | ||
| thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") | ||
|
|
@@ -367,7 +367,7 @@ func NewVariablesNormalizer() *VariablesNormalizer { | |
| detectVariableUsage(&firstDetectUnused, del) | ||
|
|
||
| secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") | ||
| variablesExtractionVisitor := extractVariables(&secondExtract) | ||
| variablesExtractionVisitor := extractVariables(&secondExtract, withFieldArgMapping) | ||
| extractVariablesDefaultValue(&secondExtract) | ||
|
|
||
| fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") | ||
|
|
@@ -382,22 +382,25 @@ func NewVariablesNormalizer() *VariablesNormalizer { | |
| } | ||
| } | ||
|
|
||
| func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) []uploads.UploadPathMapping { | ||
| func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizerResult { | ||
| v.firstDetectUnused.Walk(operation, definition, report) | ||
| if report.HasErrors() { | ||
| return nil | ||
| return VariablesNormalizerResult{} | ||
| } | ||
| v.secondExtract.Walk(operation, definition, report) | ||
| if report.HasErrors() { | ||
| return nil | ||
| return VariablesNormalizerResult{} | ||
| } | ||
| v.thirdDeleteUnused.Walk(operation, definition, report) | ||
| if report.HasErrors() { | ||
| return nil | ||
| return VariablesNormalizerResult{} | ||
| } | ||
| v.fourthCoerce.Walk(operation, definition, report) | ||
|
|
||
| return v.variablesExtractionVisitor.uploadsPath | ||
| return VariablesNormalizerResult{ | ||
| UploadsMapping: v.variablesExtractionVisitor.uploadsPath, | ||
| FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping.result, | ||
| } | ||
| } | ||
|
|
||
| type fragmentCycleVisitor struct { | ||
|
|
||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.