Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type comparison missing from FieldsWillMerge validation #4403

Open
gmac opened this issue Mar 24, 2023 · 0 comments
Open

Type comparison missing from FieldsWillMerge validation #4403

gmac opened this issue Mar 24, 2023 · 0 comments
Milestone

Comments

@gmac
Copy link
Contributor

gmac commented Mar 24, 2023

Describe the bug

FieldsWillMerge.find_conflict omits the type parity check implemented by graphql-js. This deviates the GraphQL Ruby implementation from spec compliance. Unfortunately, fixing this issue is a breaking change because existing queries will start getting rejected with a validation error.

This issue has been discussed and some work has been done. Steps to resolution:

GraphQL schema

class TestSchema < GraphQL::Schema
  class Decimal < GraphQL::Schema::Scalar
    graphql_name "Decimal"
  end

  class MoneyV2 < GraphQL::Schema::Object
    graphql_name "MoneyV2"
    field :amount, Decimal, null: false
  end

  class PricingPercentageValue < GraphQL::Schema::Object
    graphql_name "PricingPercentageValue"
    field :percentage, Float, null: false
  end

  class PricingValue < GraphQL::Schema::Union
    graphql_name "PricingValue"
    possible_types MoneyV2, PricingPercentageValue

    def self.resolve_type(obj, ctx)
      {
        "MoneyV2" => MoneyV2,
        "PricingPercentageValue" => PricingPercentageValue,
      }.fetch(obj[:__typename])
    end
  end

  class Query < GraphQL::Schema::Object
    graphql_name "Query"
    field :pricing_values, [PricingValue], null: false

    def pricing_values
      [
        { amount: "1.0", __typename: "MoneyV2" },
        { percentage: 1.0, __typename: "PricingPercentageValue" }
      ]
    end
  end

  query Query
end

GraphQL query

query = <<~GRAPHQL
  {
    pricingValues {
      __typename
      ...on MoneyV2 {
        discountValue: amount
      }
      ...on PricingPercentageValue {
        discountValue: percentage
      }
    }
  }
GRAPHQL

result = TestSchema.execute(query, validate: true)
pp result.to_h

Expected behavior

The following error is logged for this same composition from graphql-js. It's triggered by the types check in OverlappingFieldsCanBeMergedRule, which is not implemented in GraphQL Ruby.

{
  "errors": [
    {
      "message": "Fields \"discountValue\" conflict because they return conflicting types \"Decimal!\" and \"Float!\". Use different aliases on the fields to fetch both if this was intentional.",
      "locations": [
        {
          "line": 5,
          "column": 7
        },
        {
          "line": 8,
          "column": 7
        }
      ]
    }
  ]
}

Actual behavior

This test succeeds without validation error, because the relevant rules are missing from GraphQL Ruby:

{"data"=>
  {"pricingValues"=>
    [{"__typename"=>"MoneyV2", "discountValue"=>"1.0"},
     {"__typename"=>"PricingPercentageValue", "discountValue"=>1.0}]}}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants