blueprinter icon indicating copy to clipboard operation
blueprinter copied to clipboard

Make field naming more consistent

Open jasonkarns opened this issue 3 years ago • 13 comments

Fields that are simply "renamed" use: field :internal_name, name: :output_name

But fields that are defined "inline" use field(:output_name) { ... }

In the former case, the argument to field is not the final output name, but in the latter case it is. I find this inconsistency a bit jarring. I understand why it is this way, but it feels backwards to me.

What would be the appetite for an API that did something like: (method name adopted from graphql-ruby's parameter for the same behavior: essentially aliasing.)

field :out_name, method: :internal_name

This way the parameter to field is consistently the resulting key, regardless if the field's key name remains unchanged; if it is renamed, or if it is defined directly in the blueprint.

jasonkarns avatar Sep 21 '21 15:09 jasonkarns

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Nov 01 '23 01:11 github-actions[bot]

I came here to suggest exactly this.

When I look at the serializer, I want to be able to quickly see the keys of the resulting JSON object at a glance, without having to look at options.

I am happy to open a PR with this feature, but would like to know whether you would accept this idea first.

The API suggested by @jasonkarns would allow for a backward-compatible change: using the old name: keyword would still work, but be marked deprecated, and the method: keyword (or something else, maybe more generic to not prescribe the method of extraction).

franzliedke avatar Jan 12 '24 10:01 franzliedke

@jasonkarns this is a good suggestion - would you be willing to raise a PR for it? Let's ensure backward compatibility is maintained though with a deprecation notice for the older approach.

ritikesh avatar Jan 24 '24 05:01 ritikesh

I'm also happy to work on such a PR. 😇

franzliedke avatar Jan 26 '24 14:01 franzliedke

I agree that this proposal makes more sense than what we have today. I know @ritikesh suggested deprecating the old way, but that implies eventually removing the old way, and that could be a significant challenge for large code bases.

To avoid that, what does everyone think about supporting both ways? (Which to give credit where credit's due, was @njbbaer's idea iirc).

field :foop, name: :foop  # a field called "foo" that pulls from a "foop" method
field :bar, method: :barp # a field called "bar" that pulls from a "barp" method

Granted it's a bit confusing using them right beside each other, but that's on whoever writes the code. Presumably an entire app, or at least an entire view or Blueprint, would choose one way and stick with it.

Also, I don't love using method since there's a method instance method on Object. Can anyone think of a better name?

jhollinger avatar Jan 26 '24 20:01 jhollinger

I think we should allow both for now, with plans to deprecate later at v2.0 and provide a migration script for larger codebases. Is anyone familiar enough with those types of scripts in Ruby to create that or gauge level of effort?

njbbaer avatar Jan 29 '24 23:01 njbbaer

How about accessor, attribute, or source as an alternative to method?

njbbaer avatar Jan 30 '24 06:01 njbbaer

Deprecations

I know @ritikesh suggested deprecating the old way, but that implies eventually removing the old way, and that could be a significant challenge for large code bases. To avoid that, what does everyone think about supporting both ways? (Which to give credit where credit's due, was @njbbaer's idea iirc).

Given that 1.0 was released just a few weeks ago, I think it's absolutely legitimate to announce a deprecation now and remove the old variant in 2.0 whenever that gets shipped one day. Especially considering that the deprecated instances are relatively easy to find (or even automate as you suggested).

Supporting both just invites confusion, especially assuming the deprecated behavior wouldn't be documented anymore.

Naming

  • method: not ideal, as sending a message / calling a method is just one possible way of extraction - it wouldn't make sense for hashes
  • accessor: same problem IMO, it hints at a message being called

My vote is on @njbbaer's suggestion of source.

franzliedke avatar Feb 02 '24 11:02 franzliedke

@franzliedke pls feel free to do so.

I'm also happy to work on such a PR. 😇

ritikesh avatar Feb 05 '24 10:02 ritikesh

Hello everyone.

Can I get some input on the open questions on my PR #393? 🙏🏼 I would love to finalize this.

franzliedke avatar Feb 27 '24 19:02 franzliedke

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Apr 28 '24 01:04 github-actions[bot]

I got the replies I needed and will get back to my PR soon. Currently away from my computer for a few more weeks…

franzliedke avatar Apr 28 '24 18:04 franzliedke