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

Update GraphqlInputObject compiler to skip methods from code #1369

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

jeffcarbs
Copy link
Contributor

Motivation

We've come across some graphql input objects that define a method with the same name as an argument. For example:

class SomeInput < ::GraphQL::Schema::InputObject
  argument :foo_id, String

  sig { returns(Integer) }
  def foo_id
    self[:foo_id].to_i
  end
end

In reality, what happens is that the graphql gem meta-programmatically defines a foo_id method (source) and then the user-defined foo_id method overrides it. If the sigs are different (like in the example above) this confuses sorbet so we should avoid defining RBI for any arguments where an explicit getter is already defined in code.

Implementation

I updated the GraphqlInputObject compiler to only generate RBI for methods defined via the graphql gem.

Tests

See added tests.

@jeffcarbs jeffcarbs requested a review from a team as a code owner January 23, 2023 17:43
Any method defined by `GraphQL::Schema::InputObject` will end up
having a source file that points to the same file that defines the
`GraphQL::Schema::InputObject.argument` method. So, we can simplify
the check for whether a method is defined by
`GraphQL::Schema::InputObject` or if that method was redefined later,
by comparing the source file of the method to the source file of the
`GraphQL::Schema::InputObject.argument` method.
@paracycle paracycle merged commit 5c4687c into Shopify:main Feb 15, 2023
@jeffcarbs jeffcarbs deleted the graphql-input-objects branch February 15, 2023 15:52
@shopify-shipit shopify-shipit bot temporarily deployed to production February 21, 2023 12:01 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants