OperationBuilder#operations_payload messes with keys

[julianrubisch]

While working on a specific StimulusReflex, I was experiencing a weird bug, where underscored payload keys would come back dasherized.

So self.payload = {"foo_bar" => "baz"} on the JS came back as {"foo-bar" => "baz"}

I found out that this probably relates to OperationBuilder#operations_payload deep-camelizing all keys:

https://github.com/stimulusreflex/cable_ready/blob/master/lib/cable_ready/operation_builder.rb#L71-L73

The relevant test says this is a feature:

https://github.com/stimulusreflex/cable_ready/blob/master/test/lib/cable_ready/operation_builder_test.rb#L90-L94

However, it can lead to a confusing DX specifically in combination with StimulusReflex payloads. We could fix this maybe by explicitly opting out of the :payload hash key in operations_payload, although I'm a bit torn on this one.

Thoughts @leastbad @hopsoft ?

[leastbad]

Weird! I'm surprised this wasn't picked up sooner.

My first reaction is that I agree with the test: keys should be camelized.

However, what you're showing here is that payload keys are showing up dasherized aka kebab-case, which I agree is a bug or at least not useful or desireable.

So...

1. Do you agree that keys should be camelized?
2. Do you see how/why/where the dasherizing is happening?

   "my_bad".to_s.camelize(:lower) # => "myBad"

   I don't see anything in operations_payload that would dasherize, do you?

[julianrubisch]

Agreed, but if my self.payload constructs keys as underscored, am I not right in expecting them as underscored on the JS side?

Maybe I'm overthinking this though.

[leastbad]

We're talking about two issues at once.

We have a dasherize bug that we need to track down.

As for key conversion... I sort of do think that there's a convention in place, no? We camelize keys all across both libraries, already - especially on the CR side.

Here's the thing: it's not like we're breaking something that's expecting snake case. SR payload is a new concept, so presumably everyone working with it is writing code for it. If we tell them that keys get camelized - certainly the standard in the JS world - then that's just library syntax.

That's one position. Can you make a case for figuring out how to not modify the keys? I'm open to changing my mind!

[julianrubisch]

I'll go hunting that dasherize bug but I indeed have a use case where I'd like to reuse a string helper function on the JS side.

There's certainly a point to be made about conventions. I'll give this some more thought.

[leastbad]

Just to follow-up, did you ever track down what was causing your snake case keys to become dasherized?

I wasn't able to reproduce this behaviour on master; self.payload = {"foo_bar" => "baz"} results in payload: {fooBar: 'baz'} on the client, as expected. If I had to take a wild swing, one of the early client package builds had something funky.

As for the question about whether JSON object keys should be camelCased, it seems as though the world has decided yes for us. What I would do, if converting the key back from camel to snake isn't possible - perhaps your keys have irregular upcase letters, for example - is:

self.payload = {"foo_bar" => "baz"}.to_json

Then you can unpack the result on the other end with JSON.parse(). I know that might be less sexy, but I hope it's a workable outcome.