Skip to content

Commit

Permalink
Allow directives without visitors (graph-gophers#591)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelnikolov authored and KNiepok committed Feb 28, 2023
1 parent 781cc50 commit 58c7052
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 43 deletions.
39 changes: 22 additions & 17 deletions graphql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,27 @@ func TestCustomDirective(t *testing.T) {
}
`,
},
{
Schema: graphql.MustParseSchema(`
# this test ensures that directives without a Go visitor counterpart are allowed
directive @awesome on FIELD_DEFINITION
type Query {
hello: String! @awesome
}`,
&helloResolver{},
),
Query: `
{
hello
}
`,
ExpectedResult: `
{
"hello": "Hello world!"
}
`,
},
})
}

Expand Down Expand Up @@ -614,23 +635,7 @@ func TestParseSchemaWithInvalidCustomDirectives(t *testing.T) {
Args args
Want want
}{
"Missing required directive": {
Args: args{
Resolver: &helloSnakeResolver1{},
Schema: `
directive @customDirective on FIELD_DEFINITION
schema {
query: Query
}
type Query {
hello_html: String! @customDirective
}
`,
},
Want: want{Error: `no visitors have been registered for directive "customDirective"`},
},

"Duplicate directive implementations": {
Args: args{
Directives: []directives.Directive{&customDirectiveVisitor{}, &customInvalidDirective{}},
Expand Down
26 changes: 0 additions & 26 deletions internal/exec/resolvable/resolvable.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,32 +283,6 @@ func applyDirectives(s *ast.Schema, visitors []directives.Directive) (map[string
byName[name] = v
}

for name, def := range s.Directives {
// TODO: directives other than FIELD_DEFINITION also need to be supported, and later addition of
// capabilities to 'visit' other kinds of directive locations shouldn't break the parsing of existing
// schemas that declare those directives, but don't have a visitor for them?
var acceptedType bool
for _, l := range def.Locations {
if l == "FIELD_DEFINITION" {
acceptedType = true
break
}
}

if !acceptedType {
continue
}

if _, ok := byName[name]; !ok {
if name == "include" || name == "skip" || name == "deprecated" || name == "specifiedBy" {
// Special case directives, ignore
continue
}

return nil, fmt.Errorf("no visitors have been registered for directive %q", name)
}
}

return byName, nil
}

Expand Down

0 comments on commit 58c7052

Please sign in to comment.