-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Description
Describe the bug
Dataloader#with has two code paths, selected by RUBY_VERSION < "3" || RUBY_ENGINE != "ruby":
# Non-MRI or Ruby < 3:
def with(source_class, *batch_args)
batch_key = source_class.batch_key_for(*batch_args)
@source_cache[source_class][batch_key] ||= begin
source = source_class.new(*batch_args)
source.setup(self)
source
end
end
# MRI Ruby 3+:
def with(source_class, *batch_args, **batch_kwargs)
batch_key = source_class.batch_key_for(*batch_args, **batch_kwargs)
@source_cache[source_class][batch_key] ||= begin
source = source_class.new(*batch_args, **batch_kwargs)
source.setup(self)
source
end
endThe comment says "truffle-ruby wasn't doing well with the implementation below", but this check also excludes JRuby 10, which handles **kwargs correctly (it targets Ruby 3.4 compatibility).
On the non-MRI path, keyword arguments are collapsed into a Hash inside *batch_args, then passed as a positional argument to Source.new. This breaks any Source subclass that uses keyword-only parameters in initialize, when run on JRuby.
Versions
graphql version: 2.5.19
rails (or other framework): N/A
other applicable versions (graphql-batch, etc): JRuby 10.0.2.0 (Ruby 3.4.2 compat)
GraphQL schema
N/A
GraphQL query
N/A
Steps to reproduce
Put this in a ruby script:
# Reproduction: graphql-ruby Dataloader#with doesn't forward kwargs on JRuby
#
# Run with: ruby tmp/graphql_dataloader_kwargs_repro.rb
#
# graphql-ruby's Dataloader#with has two code paths:
# - MRI Ruby 3+: `def with(source_class, *batch_args, **batch_kwargs)`
# - Non-MRI or Ruby < 3: `def with(source_class, *batch_args)`
#
# The non-MRI path is selected by: `RUBY_VERSION < "3" || RUBY_ENGINE != "ruby"`
#
# JRuby 10 reports RUBY_ENGINE="jruby", so it takes the non-MRI path.
# This collapses keyword arguments into a Hash in *batch_args, then
# passes it as a positional argument to Source.new -- breaking any
# Source subclass that uses keyword-only parameters.
require "graphql"
puts "RUBY_ENGINE: #{RUBY_ENGINE}"
puts "RUBY_VERSION: #{RUBY_VERSION}"
puts "graphql gem version: #{GraphQL::VERSION}"
class KeywordSource < GraphQL::Dataloader::Source
def initialize(name:, value:)
@name = name
@value = value
end
def fetch(keys)
keys.map { |k| "#{@name}:#{@value}:#{k}" }
end
end
puts "\nAttempting: dataloader.with(KeywordSource, name: 'test', value: 42).load('key1')"
begin
result = GraphQL::Dataloader.with_dataloading do |dataloader|
dataloader.with(KeywordSource, name: "test", value: 42).load("key1")
end
puts "SUCCESS: #{result.inspect}"
rescue ArgumentError => e
puts "FAILED: #{e.class}: #{e.message}"
puts "\nRoot cause: Dataloader#with uses `def with(source_class, *batch_args)`"
puts "on non-MRI engines, collapsing kwargs into a positional Hash."
puts "Then `Source.new({name: 'test', value: 42})` fails because"
puts "initialize expects keyword-only arguments."
endRun it.
Expected behavior
The results we get on MRI are expected:
RUBY_ENGINE: ruby
RUBY_VERSION: 4.0.0
graphql gem version: 2.5.19
Attempting: dataloader.with(KeywordSource, name: 'test', value: 42).load('key1')
SUCCESS: "test:42:key1"
Actual behavior
When running on JRuby 10, we get an ArgumentError:
RUBY_ENGINE: jruby
RUBY_VERSION: 3.4.2
graphql gem version: 2.5.19
Attempting: dataloader.with(KeywordSource, name: 'test', value: 42).load('key1')
FAILED: ArgumentError: wrong number of arguments (given 1, expected 0)
Root cause: Dataloader#with uses `def with(source_class, *batch_args)`
on non-MRI engines, collapsing kwargs into a positional Hash.
Then `Source.new({name: 'test', value: 42})` fails because
initialize expects keyword-only arguments.