Merge pull request #5495 from rmosolgo/reduce-field-definition-memory

Remove Field.from_options to reduce memory usage

Author: rmosolgo

.github/workflows/ci.yaml

@@ -53,7 +53,7 @@ jobs:
             ruby: 3.3
             graphql_reject_numbers_followed_by_names: 1
           - gemfile: gemfiles/rails_8.1.gemfile
-            ruby: 3.5
+            ruby: 4.0
             graphql_reject_numbers_followed_by_names: 1
             redis: 1
           - gemfile: gemfiles/rails_master.gemfile

Gemfile

@@ -15,6 +15,7 @@ end
 
 if RUBY_VERSION >= "3.2.0"
   gem "async", "~>2.0"
+  gem "minitest-mock"
 end
 
 # Required for running `jekyll algolia ...` (via `rake site:update_search_index`)

gemfiles/mongoid_8.gemfile

@@ -12,5 +12,6 @@ gem "libev_scheduler"
 gem "evt"
 gem "async"
 gem "concurrent-ruby", "1.3.4"
+gem "minitest-mock"
 
 gemspec path: "../"

gemfiles/mongoid_9.gemfile

@@ -10,5 +10,6 @@ gem "mongoid", "~> 9.0"
 gem "libev_scheduler"
 gem "evt"
 gem "async"
+gem "minitest-mock"
 
 gemspec path: "../"

gemfiles/rails_8.0.gemfile

@@ -13,5 +13,6 @@ gem "sequel"
 gem "evt"
 gem "async"
 gem "google-protobuf"
+gem "minitest-mock"
 
 gemspec path: "../"

gemfiles/rails_8.1.gemfile

@@ -13,5 +13,6 @@ gem "sequel"
 gem "evt"
 gem "async"
 gem "google-protobuf"
+gem "minitest-mock"
 
 gemspec path: "../"

gemfiles/rails_master.gemfile

@@ -22,6 +22,7 @@ gem 'puma'
 gem 'sprockets-rails'
 gem 'capybara'
 gem 'selenium-webdriver'
+gem "minitest-mock"
 
 gemspec path: "../"
 

graphql.gemspec

@@ -37,7 +37,6 @@ Gem::Specification.new do |s|
 
   s.add_development_dependency "minitest"
   s.add_development_dependency "minitest-focus"
-  s.add_development_dependency "minitest-mock"
   s.add_development_dependency "minitest-reporters"
   s.add_development_dependency "ostruct"
   s.add_development_dependency "rake"

lib/graphql/schema/field.rb

@@ -109,52 +109,6 @@ def subscription_scope
       end
       attr_writer :subscription_scope
 
-      # Create a field instance from a list of arguments, keyword arguments, and a block.
-      #
-      # This method implements prioritization between the `resolver` or `mutation` defaults
-      # and the local overrides via other keywords.
-      #
-      # It also normalizes positional arguments into keywords for {Schema::Field#initialize}.
-      # @param resolver [Class] A {GraphQL::Schema::Resolver} class to use for field configuration
-      # @param mutation [Class] A {GraphQL::Schema::Mutation} class to use for field configuration
-      # @param subscription [Class] A {GraphQL::Schema::Subscription} class to use for field configuration
-      # @return [GraphQL::Schema:Field] an instance of `self`
-      # @see {.initialize} for other options
-      def self.from_options(name = nil, type = nil, desc = nil, comment: nil, resolver: nil, mutation: nil, subscription: nil,**kwargs, &block)
-        if (resolver_class = resolver || mutation || subscription)
-          # Add a reference to that parent class
-          kwargs[:resolver_class] = resolver_class
-        end
-
-        if name
-          kwargs[:name] = name
-        end
-
-        if comment
-          kwargs[:comment] = comment
-        end
-
-        if !type.nil?
-          if desc
-            if kwargs[:description]
-              raise ArgumentError, "Provide description as a positional argument or `description:` keyword, but not both (#{desc.inspect}, #{kwargs[:description].inspect})"
-            end
-
-            kwargs[:description] = desc
-            kwargs[:type] = type
-          elsif (resolver || mutation) && type.is_a?(String)
-            # The return type should be copied from the resolver, and the second positional argument is the description
-            kwargs[:description] = type
-          else
-            kwargs[:type] = type
-          end
-          if type.is_a?(Class) && type < GraphQL::Schema::Mutation
-            raise ArgumentError, "Use `field #{name.inspect}, mutation: Mutation, ...` to provide a mutation to this field instead"
-          end
-        end
-        new(**kwargs, &block)
-      end
-
       # Can be set with `connection: true|false` or inferred from a type name ending in `*Connection`
       # @return [Boolean] if true, this field will be wrapped with Relay connection behavior
       def connection?

lib/graphql/schema/member/has_fields.rb

@@ -6,10 +6,42 @@ class Member
       # Shared code for Objects, Interfaces, Mutations, Subscriptions
       module HasFields
         # Add a field to this object or interface with the given definition
-        # @see {GraphQL::Schema::Field#initialize} for method signature
+        # @param name_positional [Symbol] Keyword `name:` also supported
+        # @param type_positional [Class, Array<Class>] Keyword `type:` also supported
+        # @param desc_positional [String] Keyword `description:` also supported
+        # @see {GraphQL::Schema::Field#initialize} for keywords
         # @return [GraphQL::Schema::Field]
-        def field(*args, **kwargs, &block)
-          field_defn = field_class.from_options(*args, owner: self, **kwargs, &block)
+        def field(name_positional = nil, type_positional = nil, desc_positional = nil, **kwargs, &block)
+          resolver = kwargs.delete(:resolver)
+          mutation = kwargs.delete(:mutation)
+          subscription = kwargs.delete(:subscription)
+          if (resolver_class = resolver || mutation || subscription)
+            # Add a reference to that parent class
+            kwargs[:resolver_class] = resolver_class
+          end
+
+          kwargs[:name] ||= name_positional
+          if !type_positional.nil?
+            if desc_positional
+              if kwargs[:description]
+                raise ArgumentError, "Provide description as a positional argument or `description:` keyword, but not both (#{desc_positional.inspect}, #{kwargs[:description].inspect})"
+              end
+
+              kwargs[:description] = desc_positional
+              kwargs[:type] = type_positional
+            elsif (resolver || mutation) && type_positional.is_a?(String)
+              # The return type should be copied from the resolver, and the second positional argument is the description
+              kwargs[:description] = type_positional
+            else
+              kwargs[:type] = type_positional
+            end
+
+            if type_positional.is_a?(Class) && type_positional < GraphQL::Schema::Mutation
+              raise ArgumentError, "Use `field #{name_positional.inspect}, mutation: Mutation, ...` to provide a mutation to this field instead"
+            end
+          end
+
+          field_defn = field_class.new(owner: self, **kwargs, &block)
           add_field(field_defn)
           field_defn
         end