From 9781d7ddcb25bec1a776a6c579776561fd3dab77 Mon Sep 17 00:00:00 2001 From: Alfonso Uceda Date: Wed, 18 Mar 2026 16:38:23 +0100 Subject: [PATCH 1/3] Loader.for kwargs mutation on Ruby 4.0 is corrected Ruby 4.0 shares the internal kwargs hash when `...` is forwarded to multiple call sites in the same method. In `Loader.for(...)`, the kwargs captured by `loader_key_for(...)` are emptied when `new(...)` destructures them in `initialize`, corrupting the stored loader key and breaking batch grouping. Explicit `*group_args, **group_kwargs, &block` ensures each call site receives an independent copy of the kwargs hash. --- lib/graphql/batch/loader.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/graphql/batch/loader.rb b/lib/graphql/batch/loader.rb index 0484df5..2f7bb38 100644 --- a/lib/graphql/batch/loader.rb +++ b/lib/graphql/batch/loader.rb @@ -1,10 +1,11 @@ module GraphQL::Batch class Loader - # Use new argument forwarding syntax if available as an optimization if RUBY_ENGINE && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.7") class_eval(<<~RUBY, __FILE__, __LINE__ + 1) - def self.for(...) - current_executor.loader(loader_key_for(...)) { new(...) } + def self.for(*group_args, **group_kwargs, &block) + current_executor.loader(loader_key_for(*group_args, **group_kwargs)) do + new(*group_args, **group_kwargs, &block) + end end RUBY else From bd89ca8cefe5ec58ff1be9e04b23d480b7452165 Mon Sep 17 00:00:00 2001 From: Alfonso Uceda Date: Wed, 18 Mar 2026 16:39:15 +0100 Subject: [PATCH 2/3] Ruby 3.4 and 4.0 are added to CI matrix --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index efbeff1..5433bd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: [2.7, 3.0, 3.1, 3.2, 3.3] + ruby: [2.7, 3.0, 3.1, 3.2, 3.3, 3.4, 4.0] graphql_version: ['~> 1.13', '~> 2.0'] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 From 4fb820aa460ab73e940ea47f03d736ac00979f3e Mon Sep 17 00:00:00 2001 From: Alfonso Uceda Date: Wed, 18 Mar 2026 17:12:43 +0100 Subject: [PATCH 3/3] Loader.for kwargs mutation on Ruby 4.0 regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduces a bug where kwargs forwarded through ... get mutated when destructured and re-assembled by an intermediary method (e.g. RecordLoader.load → .for(model, key: key, ...)). Direct .for() calls don't trigger this. --- test/loader_test.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/loader_test.rb b/test/loader_test.rb index e3304ee..91ea142 100644 --- a/test/loader_test.rb +++ b/test/loader_test.rb @@ -63,6 +63,21 @@ def perform(_keys) end end + class KwargsGroupCountLoader < GraphQL::Batch::Loader + def initialize(group, key: :id, preload: true, not_found_value: nil, **conditions) + end + + def perform(keys) + keys.each { |key| fulfill(key, keys.size) } + end + + # Mimics RecordLoader.load: destructures kwargs with defaults and **rest, + # then re-passes them to .for() + def self.load_via_intermediary(group, load_key, key: :id, preload: true, not_found_value: nil, **conditions) + self.for(group, key: key, preload: preload, not_found_value: not_found_value, **conditions).load(load_key) + end + end + def setup GraphQL::Batch::Executor.current = GraphQL::Batch::Executor.new end @@ -92,6 +107,27 @@ def test_query_group assert_equal [2, 1, 2], group.sync end + # Reproduces a Ruby 4.0 bug where kwargs forwarded through ... get mutated + # when they were destructured and re-assembled by an intermediary method. + # Direct .for() calls don't trigger this; the kwargs must pass through a + # method that destructures them into named parameters (with defaults and + # **rest) then re-passes them to .for(). + def test_query_group_with_kwargs_via_intermediary + group = Promise.all([ + KwargsGroupCountLoader.load_via_intermediary('two', :a), + KwargsGroupCountLoader.load_via_intermediary('one', :a), + KwargsGroupCountLoader.load_via_intermediary('two', :b), + ]) + assert_equal [2, 1, 2], group.sync + end + + def test_loader_key_not_mutated_by_for_via_intermediary + KwargsGroupCountLoader.load_via_intermediary('test', :a).sync + loader = GraphQL::Batch::Executor.current.instance_variable_get(:@loaders).values.first + expected_kwargs = { key: :id, preload: true, not_found_value: nil } + assert_equal expected_kwargs, loader.loader_key[1] + end + def test_query_many assert_equal [:a, :b, :c], EchoLoader.load_many([:a, :b, :c]).sync end