Skip to content

Commit fa544f1

Browse files
ysmolskiysmolski
committed
fix: add an option to ignore skip/include directives
During normalization this option enforces directiveIncludeSkipVisitor to keep nodes and delete directives instead.
1 parent f6157d6 commit fa544f1

File tree

3 files changed

+115
-19
lines changed

3 files changed

+115
-19
lines changed

‎v2/pkg/astnormalization/astnormalization.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ type options struct {
150150
removeUnusedVariables bool
151151
removeNotMatchingOperationDefinitions bool
152152
normalizeDefinition bool
153+
ignoreSkipInclude bool
153154
}
154155

155156
type Option func(options *options)
@@ -190,6 +191,12 @@ func WithNormalizeDefinition() Option {
190191
}
191192
}
192193

194+
func WithIgnoreSkipInclude() Option {
195+
return func(options *options) {
196+
options.ignoreSkipInclude = true
197+
}
198+
}
199+
193200
func (o *OperationNormalizer) setupOperationWalkers() {
194201
o.operationWalkers = make([]walkerStage, 0, 9)
195202

@@ -209,7 +216,7 @@ func (o *OperationNormalizer) setupOperationWalkers() {
209216

210217
directivesIncludeSkip := astvisitor.NewWalker(8)
211218
preventFragmentCycles(&directivesIncludeSkip)
212-
directiveIncludeSkip(&directivesIncludeSkip)
219+
directiveIncludeSkip(&directivesIncludeSkip, o.options.ignoreSkipInclude)
213220

214221
cleanup := astvisitor.NewWalker(8)
215222
deduplicateFields(&cleanup)

‎v2/pkg/astnormalization/directive_include_skip.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,24 @@ import (
1010
"github.com/wundergraph/graphql-go-tools/v2/pkg/lexer/literal"
1111
)
1212

13-
func directiveIncludeSkip(walker *astvisitor.Walker) {
13+
// directiveIncludeSkipDeleteNodes registers a visitor to handle @include and @skip directives.
14+
// It deletes nodes that are evaluated as unused by the directives.
15+
func directiveIncludeSkipDeleteNodes(walker *astvisitor.Walker) {
16+
directiveIncludeSkip(walker, false)
17+
}
18+
19+
// directiveIncludeSkipKeepNodes registers a visitor to handle @include and @skip directives.
20+
// It unconditionally removes the directives and keeps parent nodes.
21+
func directiveIncludeSkipKeepNodes(walker *astvisitor.Walker) {
22+
directiveIncludeSkip(walker, true)
23+
}
24+
25+
// directiveIncludeSkip registers a visitor to handle @include and @skip directives.
26+
// If keepNodes is true, it unconditionally removes the directives and keeps parent nodes.
27+
func directiveIncludeSkip(walker *astvisitor.Walker, keepNodes bool) {
1428
visitor := directiveIncludeSkipVisitor{
15-
Walker: walker,
29+
Walker: walker,
30+
keepNodes: keepNodes,
1631
}
1732
walker.RegisterEnterDocumentVisitor(&visitor)
1833
walker.RegisterEnterDirectiveVisitor(&visitor)
@@ -21,6 +36,7 @@ func directiveIncludeSkip(walker *astvisitor.Walker) {
2136
type directiveIncludeSkipVisitor struct {
2237
*astvisitor.Walker
2338
operation, definition *ast.Document
39+
keepNodes bool
2440
}
2541

2642
func (d *directiveIncludeSkipVisitor) EnterDocument(operation, definition *ast.Document) {
@@ -62,7 +78,7 @@ func (d *directiveIncludeSkipVisitor) handleSkip(ref int) {
6278
default:
6379
return
6480
}
65-
if skip {
81+
if !d.keepNodes && bool(skip) {
6682
d.handleRemoveNode()
6783
} else {
6884
d.operation.RemoveDirectiveFromNode(d.Ancestors[len(d.Ancestors)-1], ref)
@@ -91,7 +107,7 @@ func (d *directiveIncludeSkipVisitor) handleInclude(ref int) {
91107
default:
92108
return
93109
}
94-
if include {
110+
if d.keepNodes || bool(include) {
95111
d.operation.RemoveDirectiveFromNode(d.Ancestors[len(d.Ancestors)-1], ref)
96112
} else {
97113
d.handleRemoveNode()

‎v2/pkg/astnormalization/directive_include_skip_test.go‎

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import "testing"
44

55
func TestDirectiveIncludeVisitor(t *testing.T) {
66
t.Run("remove static include true on inline fragment", func(t *testing.T) {
7-
run(t, directiveIncludeSkip, testDefinition, `
7+
run(t, directiveIncludeSkipDeleteNodes, testDefinition, `
88
{
99
dog {
1010
name: nickname
@@ -34,7 +34,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
3434
})
3535

3636
t.Run("if node is last one replace selection with a typename", func(t *testing.T) {
37-
run(t, directiveIncludeSkip, testDefinition, `
37+
run(t, directiveIncludeSkipDeleteNodes, testDefinition, `
3838
{
3939
dog {
4040
... @include(if: false) {
@@ -55,7 +55,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
5555
}`)
5656
})
5757
t.Run("include variables true", func(t *testing.T) {
58-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
58+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
5959
query($yes: Boolean!) {
6060
dog {
6161
... @include(if: $yes) {
@@ -78,7 +78,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
7878
}`, `{"yes":true}`)
7979
})
8080
t.Run("include variables false", func(t *testing.T) {
81-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
81+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
8282
query($no: Boolean!) {
8383
dog {
8484
... @include(if: $no) {
@@ -99,7 +99,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
9999
}`, `{"no":false}`)
100100
})
101101
t.Run("include variables mixed", func(t *testing.T) {
102-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
102+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
103103
query($yes: Boolean!, $no: Boolean!) {
104104
dog {
105105
... @include(if: $yes) {
@@ -120,7 +120,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
120120
}`, `{"yes":false,"no":true}`)
121121
})
122122
t.Run("skip variables true", func(t *testing.T) {
123-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
123+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
124124
query($yes: Boolean!) {
125125
dog {
126126
... @skip(if: $yes) {
@@ -141,7 +141,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
141141
}`, `{"yes":true}`)
142142
})
143143
t.Run("skip variables false", func(t *testing.T) {
144-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
144+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
145145
query($no: Boolean!) {
146146
dog {
147147
... @skip(if: $no) {
@@ -164,7 +164,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
164164
}`, `{"no":false}`)
165165
})
166166
t.Run("skip variables mixed", func(t *testing.T) {
167-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
167+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
168168
query($yes: Boolean!, $no: Boolean!) {
169169
dog {
170170
... @skip(if: $yes) {
@@ -185,7 +185,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
185185
}`, `{"yes":true,"no":false}`)
186186
})
187187
t.Run("skip include variables mixed", func(t *testing.T) {
188-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
188+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
189189
query($yes: Boolean!, $no: Boolean!) {
190190
dog {
191191
... @skip(if: $yes) {
@@ -206,7 +206,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
206206
}`, `{"yes":true,"no":false}`)
207207
})
208208
t.Run("skip include variables mixed reverse", func(t *testing.T) {
209-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
209+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
210210
query($yes: Boolean!, $no: Boolean!) {
211211
dog {
212212
... @include(if: $yes) {
@@ -229,7 +229,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
229229
}`, `{"yes":true,"no":false}`)
230230
})
231231
t.Run("skip should respect default values for variables", func(t *testing.T) {
232-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
232+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
233233
query($yes: Boolean = true, $no: Boolean = false) {
234234
dog {
235235
... @skip(if: $yes) {
@@ -250,7 +250,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
250250
}`, `{}`)
251251
})
252252
t.Run("include should respect default values for variables", func(t *testing.T) {
253-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
253+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
254254
query($yes: Boolean = true, $no: Boolean = false) {
255255
dog {
256256
... @include(if: $yes) {
@@ -273,7 +273,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
273273
}`, `{}`)
274274
})
275275
t.Run("skip should respect values over default values for variables", func(t *testing.T) {
276-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
276+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
277277
query($yes: Boolean = false, $no: Boolean = true) {
278278
dog {
279279
... @skip(if: $yes) {
@@ -294,7 +294,7 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
294294
}`, `{"yes":true,"no":false}`)
295295
})
296296
t.Run("include should respect values over default values for variables", func(t *testing.T) {
297-
runWithVariables(t, directiveIncludeSkip, testDefinition, `
297+
runWithVariables(t, directiveIncludeSkipDeleteNodes, testDefinition, `
298298
query($yes: Boolean = false, $no: Boolean = true) {
299299
dog {
300300
... @include(if: $yes) {
@@ -316,4 +316,77 @@ func TestDirectiveIncludeVisitor(t *testing.T) {
316316
}
317317
}`, `{"yes":true,"no":false}`)
318318
})
319+
320+
t.Run("keepNodes", func(t *testing.T) {
321+
322+
t.Run("skip should keep nodes", func(t *testing.T) {
323+
runWithVariables(t, directiveIncludeSkipKeepNodes, testDefinition, `
324+
query($yes: Boolean = false, $no: Boolean = true) {
325+
dog {
326+
... @skip(if: $yes) {
327+
includeName: name
328+
}
329+
}
330+
withAlias: dog {
331+
name @skip(if: $no)
332+
}
333+
}`, `
334+
query($yes: Boolean = false, $no: Boolean = true) {
335+
dog {
336+
... {
337+
includeName: name
338+
}
339+
}
340+
withAlias: dog {
341+
name
342+
}
343+
}`, `{"yes":true,"no":false}`)
344+
})
345+
t.Run("include should keep nodes", func(t *testing.T) {
346+
runWithVariables(t, directiveIncludeSkipKeepNodes, testDefinition, `
347+
query($yes: Boolean = false, $no: Boolean = true) {
348+
dog {
349+
... @include(if: $yes) {
350+
includeName: name
351+
}
352+
}
353+
withAlias: dog {
354+
name @include(if: $no)
355+
}
356+
}`, `
357+
query($yes: Boolean = false, $no: Boolean = true) {
358+
dog {
359+
... {
360+
includeName: name
361+
}
362+
}
363+
withAlias: dog {
364+
name
365+
}
366+
}`, `{"yes":true,"no":false}`)
367+
})
368+
t.Run("include/skip should keep nodes using default values", func(t *testing.T) {
369+
runWithVariables(t, directiveIncludeSkipKeepNodes, testDefinition, `
370+
query($yes: Boolean = false, $no: Boolean = true) {
371+
dog {
372+
... @include(if: $yes) {
373+
includeName: name
374+
}
375+
}
376+
withAlias: dog {
377+
name @skip(if: $no)
378+
}
379+
}`, `
380+
query($yes: Boolean = false, $no: Boolean = true) {
381+
dog {
382+
... {
383+
includeName: name
384+
}
385+
}
386+
withAlias: dog {
387+
name
388+
}
389+
}`, `{}`)
390+
})
391+
})
319392
}

0 commit comments

Comments
 (0)