Camelize all props that can be converted to JSON

[j-clark]

Summary

This solves the case where we want to camelize_props, but those props aren't explicitly a hash or array (for example ActiveModel::Serializer, Representable, ActionController::Parameters, etc.

As mentioned about, I'm making the assumption that ActiveSupport is loaded since this is within Rails, but let me know if that's not a valid assumption

Also, thoughts on simplifying it even further? If we know that ActiveSupport is defined, we could just use #deep_transform_keys as so

def self.camelize_props(props)
  props_as_json = props.as_json

  case props_as_json
  when Hash
    props_as_json.deep_transform_keys { |key| key.to_s.camelize(:lower) }
  when Array
    props.map { |prop| camelize_props(props) }
  else
    props
  end
end

[j-clark]

Any opinions on this?

[BookOfGreg]

It's a good upgrade over the existing params check.
Was thinking if there were any negative side effects of calling as_json on everything but can't think of one, all major Ruby and Rails enumerable objects likely to be passed in do declare that method as far as I know.

The doc for that methods needs to be updated to reflect this change and I need to think if this classes as a breaking change or if I can put it in a feature level patch.

Good contribution, glad that GitHub added the bookmark notifications tab as to be honest this one got forgotten about a little (sorry!)

Edit:
ActiveSupport will be defined but we can't guarantee the version, was deep_transform_keys added after RoR v3.2?

[j-clark]

ah, good call on version. as far as i can tell, it looks like deep_transform_keys was added in rails 4. i think as_json goes back to 2.3 (according to apidock). i don't know if they were necessarily added at the same time, but i'm pretty sure that anything responding to to_json (which is already called in the view helper) will also respond to as_json

i think putting this in a patch version is probably too risky. someone is inevitable passing in active record objects, which would get serialized as json correctly, but not camelized. then getting this update would likely break their apps. and this is kind of tricky to track down.

i'll update the docs later today. is it just the README?

thanks!

[BookOfGreg]

Inline docs here.

[BookOfGreg]

Marked the inline doc as a comment.
Additions to the readme and/or wiki are always very welcome as they do talk about camelization in general which would be changing.

I was wondering if there was some sort of option we could put this behind to make it opt-in, thereby letting us release as a feature without breaking people's existing React apps. However since there's already code there to specifically change Params, any toggle ignoring params would be a very odd toggle indeed.

Likely at this point to make it hold off for a major version bump. Need to update pre-bundled react anyway 👍

[justin808]

I think this is OK to merge as-is.

@BookOfGreg @j-clark any thoughts on this?

@j-clark can you please add a changelog entry?

[justin808]

LGTM

[alkesh26]

We need to add a CHANGELOG entry, and the PR is good to go.

[j-clark]

wow, this PR is a relic from the past! i'll be honest, it's unlikely that i'll get a changelog entry added any time soon. i'm a corporate drone these days and don't even have git setup on my work laptop 😞

[Judahmeek]

@justin808 I created a separate PR with the changelog entry for this PR: #1207

Let's get this merged.