Directives with .visible? -> false are not fully removed from the schema

[erikkessler1]

Describe the bug

I'm trying to make it so that certain directives of a schema are only present in certain contexts (e.g., federation directives only included in federated contexts). From playing around with the .visible? hook, it seems like a directive that isn't visible has its definition removed, but the usage of the directive still exists. For example, the following script:

require 'bundler'
require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'graphql', '2.4.4'
end

class MyDirective < GraphQL::Schema::Directive
  locations OBJECT

  def self.visible?(context)
    context.fetch(:show_directives)
  end
end

class Query < GraphQL::Schema::Object
  directive MyDirective
  field :my_string, String, null: false
end

class MySchema < GraphQL::Schema
  use GraphQL::Schema::Visibility
  query Query
end

puts "# show_directives: true"
puts MySchema.to_definition(context: { show_directives: true })
puts "\n# show_directives: false"
puts MySchema.to_definition(context: { show_directives: false })

Produces the following output:

# show_directives: true
directive @myDirective on OBJECT

type Query @myDirective {
  myString: String!
}

# show_directives: false
type Query @myDirective {
  myString: String!
}

The output I would expect is:

# show_directives: false
type Query {
  myString: String!
}

Is that the expected behavior of the .visible? hook on directives?

[rmosolgo]

Hey, thanks for the detailed report. I would also expect that type Query would not have @myDirective in this case. I'm guessing it just wasn't implemented yet.

I think this is the call that needs to be corrected:

graphql-ruby/lib/graphql/language/document_from_schema_definition.rb

Line 352 in 692b58a

member.public_send(directives_method).map do |dir|

It should skip over directives that fail a @types.directive_exists?(directive_name) check, similar to the one this code uses:

graphql-ruby/lib/graphql/static_validation/rules/directives_are_defined.rb

Line 10 in 692b58a

if !@types.directive_exists?(node.name)

That would remove any directives that had been hidden.

The "top-level" list of directives already uses @types.directives which excludes any hidden ones:

graphql-ruby/lib/graphql/language/document_from_schema_definition.rb

Line 266 in 692b58a

dirs_to_build = @types.directives