Merge pull request #5055 from rmosolgo/lazy-root-types

Accept blocks to reference schema root types

Author: rmosolgo

lib/graphql/rubocop.rb

@@ -3,3 +3,4 @@
 require "graphql/rubocop/graphql/default_null_true"
 require "graphql/rubocop/graphql/default_required_true"
 require "graphql/rubocop/graphql/field_type_in_block"
+require "graphql/rubocop/graphql/root_types_in_block"

lib/graphql/rubocop/graphql/root_types_in_block.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+require_relative "./base_cop"
+
+module GraphQL
+  module Rubocop
+    module GraphQL
+      # Identify (and auto-correct) any root types in your schema file.
+      #
+      # @example
+      #   # bad, immediately causes Rails to load `app/graphql/types/query.rb`
+      #   query Types::Query
+      #
+      #   # good, defers loading until the file is needed
+      #   query { Types::Query }
+      #
+      class RootTypesInBlock < BaseCop
+        MSG = "type configuration can be moved to a block to defer loading the type's file"
+
+        def_node_matcher :root_type_config_without_block, <<-Pattern
+        (
+          send nil? {:query :mutation :subscription} const
+        )
+        Pattern
+
+        def on_send(node)
+          root_type_config_without_block(node) do
+            add_offense(node) do |corrector|
+              new_node_source = node.source_range.source
+              new_node_source.sub!(/(query|mutation|subscription)/, '\1 {')
+              new_node_source << " }"
+              corrector.replace(node, new_node_source)
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/graphql/schema.rb

@@ -430,44 +430,60 @@ def connections
         end
       end
 
-      def query(new_query_object = nil)
-        if new_query_object
+      def query(new_query_object = nil, &lazy_load_block)
+        if new_query_object || block_given?
           if @query_object
-            raise GraphQL::Error, "Second definition of `query(...)` (#{new_query_object.inspect}) is invalid, already configured with #{@query_object.inspect}"
+            dup_defn = new_query_object || yield
+            raise GraphQL::Error, "Second definition of `query(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@query_object.inspect}"
+          elsif use_schema_subset?
+            @query_object = block_given? ? lazy_load_block : new_query_object
           else
-            @query_object = new_query_object
-            add_type_and_traverse(new_query_object, root: true) unless use_schema_subset?
-            nil
+            @query_object = new_query_object || lazy_load_block.call
+            add_type_and_traverse(@query_object, root: true)
           end
+          nil
+        elsif @query_object.is_a?(Proc)
+          @query_object = @query_object.call
         else
           @query_object || find_inherited_value(:query)
         end
       end
 
-      def mutation(new_mutation_object = nil)
-        if new_mutation_object
+      def mutation(new_mutation_object = nil, &lazy_load_block)
+        if new_mutation_object || block_given?
           if @mutation_object
-            raise GraphQL::Error, "Second definition of `mutation(...)` (#{new_mutation_object.inspect}) is invalid, already configured with #{@mutation_object.inspect}"
+            dup_defn = new_mutation_object || yield
+            raise GraphQL::Error, "Second definition of `mutation(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@mutation_object.inspect}"
+          elsif use_schema_subset?
+            @mutation_object = block_given? ? lazy_load_block : new_mutation_object
           else
-            @mutation_object = new_mutation_object
-            add_type_and_traverse(new_mutation_object, root: true) unless use_schema_subset?
-            nil
+            @mutation_object = new_mutation_object || lazy_load_block.call
+            add_type_and_traverse(@mutation_object, root: true)
           end
+          nil
+        elsif @mutation_object.is_a?(Proc)
+          @mutation_object = @mutation_object.call
         else
           @mutation_object || find_inherited_value(:mutation)
         end
       end
 
-      def subscription(new_subscription_object = nil)
-        if new_subscription_object
+      def subscription(new_subscription_object = nil, &lazy_load_block)
+        if new_subscription_object || block_given?
           if @subscription_object
-            raise GraphQL::Error, "Second definition of `subscription(...)` (#{new_subscription_object.inspect}) is invalid, already configured with #{@subscription_object.inspect}"
+            dup_defn = new_subscription_object || yield
+            raise GraphQL::Error, "Second definition of `subscription(...)` (#{dup_defn.inspect}) is invalid, already configured with #{@subscription_object.inspect}"
+          elsif use_schema_subset?
+            @subscription_object = block_given? ? lazy_load_block : new_subscription_object
           else
-            @subscription_object = new_subscription_object
-            add_subscription_extension_if_necessary
-            add_type_and_traverse(new_subscription_object, root: true) unless use_schema_subset?
-            nil
+            @subscription_object = new_subscription_object || lazy_load_block.call
+            add_type_and_traverse(@subscription_object, root: true)
           end
+          nil
+        elsif @subscription_object.is_a?(Proc)
+          @subscription_object = @subscription_object.call
+          add_subscription_extension_if_necessary
+          @subscription_object
         else
           @subscription_object || find_inherited_value(:subscription)
         end
@@ -1373,7 +1389,8 @@ def instrumenters
 
       # @api private
       def add_subscription_extension_if_necessary
-        if !defined?(@subscription_extension_added) && subscription && self.subscriptions
+        # TODO: when there's a proper API for extending root types, migrat this to use it.
+        if !defined?(@subscription_extension_added) && @subscription_object.is_a?(Class) && self.subscriptions
           @subscription_extension_added = true
           subscription.all_field_definitions.each do |field|
             if !field.extensions.any? { |ext| ext.is_a?(Subscriptions::DefaultSubscriptionResolveExtension) }

spec/fixtures/cop/.rubocop.yml

@@ -16,3 +16,9 @@ GraphQL/FieldTypeInBlock:
   Include:
     - field_type.rb
     - field_type_autocorrect.rb
+
+GraphQL/RootTypesInBlock:
+  Enabled: true
+  Include:
+    - root_types.rb
+    -  root_types_autocorrect.rb

spec/fixtures/cop/root_types.rb

@@ -0,0 +1,5 @@
+class MyAppSchema < GraphQL::Schema
+  query Types::Query
+  mutation Types::Mutation
+  subscription Types::Subscription
+end

spec/fixtures/cop/root_types_corrected.rb

@@ -0,0 +1,5 @@
+class MyAppSchema < GraphQL::Schema
+  query { Types::Query }
+  mutation { Types::Mutation }
+  subscription { Types::Subscription }
+end

spec/graphql/cop/root_types_in_block_spec.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe "GraphQL::Cop::RootTypesInBlock" do
+  include RubocopTestHelpers
+
+  it "finds and autocorrects field corrections with inline types" do
+    result = run_rubocop_on("spec/fixtures/cop/root_types.rb")
+    puts result
+    assert_equal 3, rubocop_errors(result)
+
+    assert_includes result, <<-RUBY
+  query Types::Query
+  ^^^^^^^^^^^^^^^^^^
+    RUBY
+
+    assert_includes result, <<-RUBY
+  mutation Types::Mutation
+  ^^^^^^^^^^^^^^^^^^^^^^^^
+    RUBY
+
+    assert_includes result, <<-RUBY
+  subscription Types::Subscription
+  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    RUBY
+
+    assert_rubocop_autocorrects_all("spec/fixtures/cop/root_types.rb")
+  end
+end

spec/graphql/schema_spec.rb

@@ -476,4 +476,24 @@ class QueryRequiredSchema < GraphQL::Schema
   it "starts with no references_to" do
     assert_equal({}, GraphQL::Schema.references_to)
   end
+
+  it "defers root type blocks until those types are used" do
+    calls = []
+    schema = Class.new(GraphQL::Schema) do
+      self.use_schema_subset = true
+      query { calls << :query; Class.new(GraphQL::Schema::Object) { graphql_name("Query") } }
+      mutation { calls << :mutation; Class.new(GraphQL::Schema::Object) { graphql_name("Mutation") } }
+      subscription { calls << :subscription; Class.new(GraphQL::Schema::Object) { graphql_name("Subscription") } }
+      # Test this because it tries to modify `subscription` -- currently hardcoded in Schema.add_subscription_extension_if_necessary
+      use GraphQL::Subscriptions
+    end
+
+    assert_equal [], calls
+    assert_equal "Query", schema.query.graphql_name
+    assert_equal [:query], calls
+    assert_equal "Mutation", schema.mutation.graphql_name
+    assert_equal [:query, :mutation], calls
+    assert_equal "Subscription", schema.subscription.graphql_name
+    assert_equal [:query, :mutation, :subscription], calls
+  end
 end

spec/graphql/subscriptions_spec.rb

@@ -205,8 +205,8 @@ class Query < GraphQL::Schema::Object
   end
 
   class Schema < GraphQL::Schema
-    query(Query)
-    subscription(Subscription)
+    query { Query }
+    subscription { Subscription }
     use InMemoryBackend::Subscriptions, extra: 123
     max_complexity(InMemoryBackend::MAX_COMPLEXITY)
   end

spec/support/dummy/schema.rb

@@ -536,9 +536,9 @@ def test; "Test"; end
   end
 
   class Schema < GraphQL::Schema
-    query DairyAppQuery
-    mutation DairyAppMutation
-    subscription Subscription
+    query { DairyAppQuery }
+    mutation { DairyAppMutation }
+    subscription { Subscription }
     max_depth 5
     orphan_types Honey
     trace_with GraphQL::Tracing::CallLegacyTracers