Fix returning [Type, false] from resolve_type

rmosolgo#4130 fixed returning
`[Type, nil]` from a union's `resolve_type`, but it caused a regression
for literal `false` values.

`Schema#resolve_type` had a `if resolved_value` conditional would be false
for `false` values causing it _always_ return a single value (the `resolve_type`).

The runtime's `resolve_type` method would then _always_ return the tuple
version (in non-lazy cases): `[type, value]`. However, since
`Schema#resolve_type` was only returning a single value, the resolved
value would be lost resulting in `[type, nil]` being passed on.

The fix is to remove the `if resolved_value` conditional entirely and
always return the two-value tuple ensuring the original resolved value
is correctly passed along.

Note: the runtime still has a branch to check if the `resolve_type`
value is a tuple or not. The only case where this won't be true is for
lazy resolved types.

Author: swalkinshaw

lib/graphql/schema.rb

@@ -785,11 +785,7 @@ def resolve_type(type, obj, ctx)
             end
 
             if resolved_type.nil? || (resolved_type.is_a?(Module) && resolved_type.respond_to?(:kind))
-              if resolved_value
-                [resolved_type, resolved_value]
-              else
-                resolved_type
-              end
+              [resolved_type, resolved_value]
             else
               raise ".resolve_type should return a type definition, but got #{resolved_type.inspect} (#{resolved_type.class}) from `resolve_type(#{type}, #{obj}, #{ctx})`"
             end

spec/graphql/schema/union_spec.rb

@@ -67,7 +67,7 @@
       assert_equal "Fragment on Ensemble can't be spread inside PerformingAct", res.to_h["errors"].first["message"]
     end
 
-    describe "two-value type resolution" do
+    describe "type resolution" do
       Box = Struct.new(:value)
 
       class Schema < GraphQL::Schema
@@ -79,60 +79,139 @@ class B < GraphQL::Schema::Object
           field :b, String, method: :itself
         end
 
-        class MyUnion < GraphQL::Schema::Union
-          possible_types A, B
+        class C < GraphQL::Schema::Object
+          field :c, Boolean, method: :itself
+        end
+
+        class UnboxedUnion < GraphQL::Schema::Union
+          possible_types A, C
 
           def self.resolve_type(object, ctx)
-            if object.value == "return-nil"
+            case object
+            when FalseClass
+              C
+            else
+              A
+            end
+          end
+        end
+
+        class BoxedUnion < GraphQL::Schema::Union
+          possible_types A, B, C
+
+          def self.resolve_type(object, ctx)
+            case object.value
+            when "return-nil"
               [B, nil]
+            when FalseClass
+              [C, object.value]
             else
               [A, object.value]
             end
           end
         end
 
         class Query < GraphQL::Schema::Object
-          field :my_union, MyUnion
+          field :boxed_union, BoxedUnion
 
-          def my_union
+          def boxed_union
             Box.new(context[:value])
           end
+
+          field :unboxed_union, UnboxedUnion
+
+          def unboxed_union
+            context[:value]
+          end
         end
 
         query(Query)
       end
 
-      it "can cast the object after resolving the type" do
+      describe "two-value resolution" do
+        it "can cast the object after resolving the type" do
 
-        query_str = <<-GRAPHQL
-        {
-          myUnion {
-            ... on A { a }
+          query_str = <<-GRAPHQL
+          {
+            boxedUnion {
+              ... on A { a }
+            }
           }
-        }
-        GRAPHQL
+          GRAPHQL
+
+          res = Schema.execute(query_str, context: { value: "unwrapped" })
+
+          assert_equal({
+            'data' => { 'boxedUnion' => { 'a' => 'unwrapped' } }
+          }, res.to_h)
+        end
+
+        it "uses `false` when returned from resolve_type" do
+          query_str = <<-GRAPHQL
+          {
+            boxedUnion {
+              ... on C { c }
+            }
+          }
+          GRAPHQL
+
+          res = Schema.execute(query_str, context: { value: false })
 
-        res = Schema.execute(query_str, context: { value: "unwrapped" })
+          assert_equal({
+            'data' => { 'boxedUnion' => { 'c' => false } }
+          }, res.to_h)
+        end
 
-        assert_equal({
-          'data' => { 'myUnion' => { 'a' => 'unwrapped' } }
-        }, res.to_h)
+        it "uses `nil` when returned from resolve_type" do
+          query_str = <<-GRAPHQL
+          {
+            boxedUnion {
+              ... on B { b }
+            }
+          }
+          GRAPHQL
+
+          res = Schema.execute(query_str, context: { value: "return-nil" })
+
+          assert_equal({
+            'data' => { 'boxedUnion' => { 'b' => nil } }
+          }, res.to_h)
+        end
       end
 
-      it "uses `nil` when returned from resolve_type" do
-        query_str = <<-GRAPHQL
-        {
-          myUnion {
-            ... on B { b }
+      describe "single-value resolution" do
+        it "can cast the object after resolving the type" do
+
+          query_str = <<-GRAPHQL
+          {
+            unboxedUnion {
+              ... on A { a }
+            }
           }
-        }
-        GRAPHQL
+          GRAPHQL
 
-        res = Schema.execute(query_str, context: { value: "return-nil" })
+          res = Schema.execute(query_str, context: { value: "string" })
 
-        assert_equal({
-          'data' => { 'myUnion' => { 'b' => nil } }
-        }, res.to_h)
+          assert_equal({
+            'data' => { 'unboxedUnion' => { 'a' => 'string' } }
+          }, res.to_h)
+        end
+
+        it "works with literal false values" do
+          query_str = <<-GRAPHQL
+          {
+            unboxedUnion {
+              ... on C { c }
+            }
+          }
+          GRAPHQL
+
+          res = Schema.execute(query_str, context: { value: false })
+
+          assert_equal({
+            'data' => { 'unboxedUnion' => { 'c' => false } }
+          }, res.to_h)
+        end
       end
     end
   end

spec/integration/rails/graphql/schema_spec.rb

@@ -61,7 +61,7 @@
     describe "when the return value is nil" do
       it "returns nil" do
         result = relay_schema.resolve_type(123, nil, GraphQL::Query::NullContext)
-        assert_equal(nil, result)
+        assert_equal([nil, nil], result)
       end
     end