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

Cop multiple field definitions #94

Conversation

toneymathews
Copy link
Contributor

Closes #90
This PR

  • adds a cop to group multiple field definitions (MFD) together. If this case, ungrouped definitions will be added following the first field definition
  • updates FieldDefinition cop. If there are MFD, the resolver method is added after the last field definition.
    • For MFD, the field node can be same. At the same time, the definitions can be different due to different arguments as in the example below
  field :image_url, String, null: false do
    argument :width, Int, null: true
  end

   field :image_url, String, null: false do
    argument :width, Int, null: true
    argument :height, Int, null: true
  end

So, RuboCop::GraphQL::Field.new now takes in the parent node if field def has a body. Associated changes are made.

There is one odd scenario as described in the example below

class UserType < BaseType
  field :first_name, String, null: true

  def first_name
     object.first_name
  end

  field :last_name, String, null: true

  field :first_name, String, null: false
  field :first_name, Name, null: false 
end

Running GraphQL/FieldDefinitions would add the first_name resolver method after the last field definition.

class UserType < BaseType
  field :first_name, String, null: true

  field :last_name, String, null: true

  field :first_name, String, null: false
  field :first_name, Name, null: false 
  
  def first_name
     object.first_name
  end
end

Then running MFD would lead to

class UserType < BaseType
  field :first_name, String, null: true
  field :first_name, String, null: false
  field :first_name, Name, null: false 

  field :last_name, String, null: true
  
  def first_name
     object.first_name
  end
end

So it's preferable to run GraphQL/MultipleFieldDefinitions before GraphQL/FieldDefinitions. I did not think too much of it since running rubocop a second time would fix this.

Copy link
Owner

@DmitryTsepelev DmitryTsepelev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you so much!

@DmitryTsepelev DmitryTsepelev merged commit 8ac92b6 into DmitryTsepelev:master Sep 28, 2022
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

Successfully merging this pull request may close these issues.

GraphQL/FieldDefinitions support for multiple definitions
2 participants