Skip to content

Commit

Permalink
Merge pull request #4807 from rmosolgo/deprecate-dataloader-lazy-resolve
Browse files Browse the repository at this point in the history
Deprecate returning `.resolve(...)` dataloader requests
  • Loading branch information
rmosolgo authored Jan 26, 2024
2 parents 930468f + 67941e5 commit 02dd62b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 48 deletions.
5 changes: 5 additions & 0 deletions lib/graphql/dataloader/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ def initialize(source, key)
def load
@source.load(@key)
end

def load_with_deprecation_warning
warn("Returning `.request(...)` from GraphQL::Dataloader is deprecated, use `.load(...)` instead. (See usage of #{@source} with #{@key.inspect}).")
load
end
end
end
end
2 changes: 1 addition & 1 deletion lib/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ def lazy_methods
else
@lazy_methods = GraphQL::Execution::Lazy::LazyMethodMap.new
@lazy_methods.set(GraphQL::Execution::Lazy, :value)
@lazy_methods.set(GraphQL::Dataloader::Request, :load)
@lazy_methods.set(GraphQL::Dataloader::Request, :load_with_deprecation_warning)
end
end
@lazy_methods
Expand Down
52 changes: 5 additions & 47 deletions spec/graphql/dataloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,7 @@ def set_cache(input:)
mutation(Mutation)

def self.object_from_id(id, ctx)
if ctx[:use_request]
ctx.dataloader.with(DataObject).request(id)
else
ctx.dataloader.with(DataObject).load(id)
end
ctx.dataloader.with(DataObject).load(id)
end

def self.resolve_type(type, obj, ctx)
Expand Down Expand Up @@ -767,12 +763,6 @@ def self.included(child_class)
[:mget, ["2", "3"]],
]
assert_equal expected_log, database_log

# Run the same test, but using `.request` from object_from_id
database_log.clear
res2 = schema.execute(query_str, context: { use_request: true })
assert_equal expected_data, res2["data"]
assert_equal expected_log, database_log
end

it "works with sources that use keyword arguments in the initializer" do
Expand Down Expand Up @@ -814,14 +804,6 @@ def self.included(child_class)
values.sort!
end
assert_equal expected_results, results.first.to_a

query2 = GraphQL::Query.new(schema, query_str, context: { use_request: true })
result2 = nil
query2.context.dataloader.run_isolated do
result2 = GraphQL::Analysis::AST.analyze_query(query2, [UsageAnalyzer])
end

assert_equal expected_results, result2.first.to_a
end

it "Works with input objects, load and request" do
Expand All @@ -846,13 +828,6 @@ def self.included(child_class)
[:mget, ["2", "3"]],
]
assert_equal expected_log, database_log


# Run the same test, but using `.request` from object_from_id
database_log.clear
res2 = schema.execute(query_str, context: { use_request: true })
assert_equal expected_data, res2["data"]
assert_equal expected_log, database_log
end

it "batches calls in .authorized?" do
Expand Down Expand Up @@ -889,13 +864,6 @@ def self.included(child_class)
[:mget, ["2", "3"]],
]
assert_equal expected_log, database_log


# Run the same test, but using `.request` from object_from_id
database_log.clear
res2 = schema.execute(query_str, context: { use_request: true }, variables: { input: { recipe1Id: 5, recipe2Id: 6 }})
assert_equal expected_data, res2["data"]
assert_equal expected_log, database_log
end

it "supports general usage" do
Expand Down Expand Up @@ -1129,15 +1097,10 @@ def fetch(ids)
class QueryType < GraphQL::Schema::Object
field :foo, Example::FooType do
argument :foo_id, GraphQL::Types::ID, required: false, loads: Example::FooType
argument :use_load, GraphQL::Types::Boolean, required: false, default_value: false
end

def foo(use_load: false, foo: nil)
if use_load
dataloader.with(Example::FooSource).load("load")
else
dataloader.with(Example::FooSource).request("request")
end
def foo(foo: nil)
dataloader.with(Example::FooSource).load("load")
end
end

Expand All @@ -1146,7 +1109,7 @@ class Schema < GraphQL::Schema
use GraphQL::Dataloader

def self.object_from_id(id, ctx)
ctx.dataloader.with(Example::FooSource).request(id)
ctx.dataloader.with(Example::FooSource).load(id)
end

def self.resolve_type(type, obj, ctx)
Expand All @@ -1158,11 +1121,7 @@ def self.resolve_type(type, obj, ctx)
it "loads properly" do
result = Example::Schema.execute(<<-GRAPHQL)
{
foo(useLoad: false, fooId: "Other") {
__typename
id
}
fooWithLoad: foo(useLoad: true, fooId: "Other") {
fooWithLoad: foo(fooId: "Other") {
__typename
id
}
Expand All @@ -1171,7 +1130,6 @@ def self.resolve_type(type, obj, ctx)
# This should not have a Lazy in it
expected_result = {
"data" => {
"foo" => { "id" => "request", "__typename" => "Foo" },
"fooWithLoad" => { "id" => "load", "__typename" => "Foo" },
}
}
Expand Down

0 comments on commit 02dd62b

Please sign in to comment.