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

🐫 the case for camelCase'd field definitions? 🐍 #4630

Closed
fwolfst opened this issue Sep 12, 2023 · 3 comments
Closed

🐫 the case for camelCase'd field definitions? 🐍 #4630

fwolfst opened this issue Sep 12, 2023 · 3 comments

Comments

@fwolfst
Copy link
Contributor

fwolfst commented Sep 12, 2023

Is your feature request related to a problem? Please describe.

We are adopting GraphQL (hence, new to this game) and seem not to be the first to stumble with the different camelCase/snake_case "translations" that happen or not happen.

To summarize, my understanding is:

  • GraphQL community favors PascalCase for Types (like Rubys does for classes) and camelCase for fields, variables and arguments (where Rubys prefer snake_case).
  • GraphQL-rubys new defaults are to generate fields and arguments in camelCase, even if they are defined as snake_case in Ruby; this can be opted out.
  • GraphQL-rubys suggests the convention to use snake_case in Rubys field and argument definitions.
  • In any case, there is an easy mapping possible (field :camelCaseField, ..., method: :snake_case_field)

If working in the Frontend for whatever reason I discover a bug with a required field (say properTitle does give weird strings but not we want), my developers reflex is to search the Rails app source code for properTitle, especially if I didnt implement GraphQL yesterday. I will likely not find anything. Depending on my knowledge of the library and my memory, I might have to browse and guess a little until I find the implementation (field :proper_title ..... def proper_title).

We thus might always define field :properTitle, ..., method: :proper_title and were wondering Why does this not automatically resolve to the snake_cased version? (doc: Field Resolution).

I did not read through all the arguments/issues made already (see below for a list), but I got the impression that the feature is generally wanted and could get approval.

If not, it would be nice to have a rationale in the documentation (and a small warning sign, explaining that snakes might transform into camels, but camels never become snakes). But for anyone not RTFM, current behavior might be unexpected.

Describe the solution you'd like

If I could wish for a solution, it would be that Field resolution try a camel-cased version of the field name if everything else fails (or earlier). I know that there is no 1-1 translation, but it should work 95% of the time; method: other_name can still be defined manually if needed. There could be a way to opt-out or opt-in.

Describe alternatives you've considered

  • following the convention and get annoyed by it in the future, or (worse) live with annoyed future colleagues
  • specify the method: and be only slightly annoyed in the present
  • implement an own field-delegating method (auto_field?) and probably hate the method later
  • override field and probably hate ourselves for it later

Additional context

References:

Afaics, simplest implementation might start here:

elsif inner_object.respond_to?(@method_sym)
.

Good first place for a snake_caseize implementation: https://github.com/rubyworks/facets/blob/main/lib/core/facets/string/snakecase.rb (+ add transliteralization if possible).

🐍 🐫 🐍 🐫

@rmosolgo
Copy link
Owner

Hi! Thanks for the detailed writeup. I can imagine the frustration not expecting the change in case between GraphQL and Ruby. I agree that there should be some documentation for this behavior -- I added a new section in 9838bbe: https://graphql-ruby.org/fields/introduction.html#field-names.

You could implement your desired behavior in your base field class:

module Types 
  class BaseField < GraphQL::Schema::Field 
    def initialize(name, *args, **kwargs, &block)
      # Try resolving this field using a camelized Ruby method, 
      # unless another `method: ...` is provided.
      kwargs[:method] ||= name.camelize
      super 
    end 
  end 
end 

How about an approach like that?

@fwolfst
Copy link
Contributor Author

fwolfst commented Sep 14, 2023

Hi! Thanks for the detailed writeup. I can imagine the frustration not expecting the change in case between GraphQL and Ruby. I agree that there should be some documentation for this behavior -- I added a new section in 9838bbe: https://graphql-ruby.org/fields/introduction.html#field-names.

You could implement your desired behavior in your base field class:

module Types 
  class BaseField < GraphQL::Schema::Field 
    def initialize(name, *args, **kwargs, &block)
      # Try resolving this field using a camelized Ruby method, 
      # unless another `method: ...` is provided.
      kwargs[:method] ||= name.camelize
      super 
    end 
  end 
end 

How about an approach like that?

The approach doesnt look too bad. Except that in our case I would like the snake_cased method to be searched, not the camelCased. I'd want to grep for every part of the schema and find its implementation with copy and paste, without having to change some typing.

Would you accept a PR that implements that behavior? If so, "where" should it be configurable? Afaics, graphql-ruby does not have a configure block or something similar. So it could be done on Schema, Type or BaseField level.

@rmosolgo
Copy link
Owner

Ah, sure -- so you'd use .underscore instead of .camelize 👍

No, I don't think we need any more changes in the gem. Field#initialize is the intended way to customize schema definition, so adding code like that to your application is your best bet!

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