stripe / stripe-ruby

Ruby library for the Stripe API.
https://stripe.com
MIT License
1.96k stars 548 forks source link

to_h should behave the same as to_hash #1008

Open mehagar opened 3 years ago

mehagar commented 3 years ago

Ruby version: ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19] Stripe gem version: 5.38.0

I was trying to convert a Stripe::StripeObject to a hash that maps symbols to strings. I called #to_h on that object, and instead of a hash from symbols to strings, I got a hash from symbols to strings and objects, where a stripe object was expanded in the response. Meanwhile, the #to_hash method did what I was expecting, and instead of storing a Ruby object as the value, converted that object to a hash of symbols to strings.

I would expect to_h to behave the same as to_hash to avoid suprising behavior.

dcr-stripe commented 3 years ago

Hi @mehagar , thanks for your report and sorry you're experiencing this issue!

You're right that to_h and to_hash should behave similarly. There's no strict requirement to have these match, but I agree the current state leads to confusion. We override to_hash here however never alias to_h. Thus, we end up using the to_h implementation from Enumerable [^1].

Unfortunately making this change is not backwards compatible, and so will have to take place in the next major version. For now I'll tag this issue with 'future' to indicate this and we'll track this work internally.

Thanks again!

[^1]: Stripe::Customer.create({...}).method(:to_h).owner === Enumerable

dcr-stripe commented 3 years ago

Just adding an extra note here that to_h as defined in Enumerable purposefully doesn't currently recurse through the entire structure converting to hashes.

As an example:

[
  [:key1, %i[hello world].each_with_index], 
  [:key2, 2]
].each_entry.to_h
=> {:key1=>#<Enumerator: [:hello, :world]:each_with_index>, :key2=>2}

There's a good discussion to be had here around what our implementation should be for to_h vs. to_hash given that to_h is used for explicit coercion and to_hash for implicit coercion.

That being said, changing either of these methods is still not backwards compatible so will wait for the next major version.