Fix have_exact_props RSpec matcher.

[bknoles]

Resolves #103

@mrsweaters reported that our recent change for deep merging broke the .have_exact_props matcher in RSpec. This change fixes that situation and cleans up the code a little bit for good measure.

Background

Until v3.1.0, Inertia Rails treated string-keyed props as different than symbol-keyed props. No one ever complained, but as I built out the deep merging feature I kept running into issues while testing with mixed string/symbol keys.

Because everything gets rendered to JSON, there's no reason we need to treat {'a' => 1} any differently than {a: 1}. As part of 3.1.0, I cast both props and shared data to HashWithIndifferentAccess in an attempt to be, well, indifferent to the way in which we write our props. We quickly found that this broke partial reloading, and we fixed that in #102. While HashWithIndifferentAccess has some really useful properties, it was not indifferent when directly comparing keys because they are treated as strings.

The issue reported in #103 is another rough edge of HashWithIndifferentAccess.

The Cause of a Broken have_exact_props

HashWithIndifferentAccess is a subclass of Hash that overwrites a bunch of the getter/setter/command methods to be ambivalent about whether strings are keys. Internally, it converts all keys to string keys. So, when you set a value like this:

indifferent_hash[:x] = 5

Under the hood, HashWithIndifferentAccess does this:

indifferent_hash['x'] = 5

Additionally, if you call .with_indifferent_access on a Hash, Ruby will traverse the entire hash and convert any symbolized keys into string keys.

$(irb): {a: 1}.with_indifferent_access => {'a' => 1}

So, HashWithIndifferentAccess converts symbol keys to string keys and overwrites Hash setters so you always get string keys.

But there's a loophole! If you emit symbolized keys within a .transform_values block, HashWithIndifferentAccess won't convert them into string keys. And you might think (as I did) that you could just call .with_indifferent_access on the resulting object to rebuild it. But HashWithIndifferentAccess.with_indifferent_access just calls .dup on the indifferent hash. It doesn't convert anything. See the source here.

And this is exactly what we were doing with lambda based shared props. Here are Inertia-less RSpec examples illustrating the bug:

describe 'surprising HashWithIndifferentAccess behavior' do
  ✅ This passes just fine
  it 'passes just fine when transforming the values of a Hash using .call' do
    string_key_hash = {'a' => ->{{b: 1}}}
    string_key_hash.transform_values!(&:call)
    symbol_key_hash = {a: {b: 1}}
    expect(string_key_hash.with_indifferent_access).to eq symbol_key_hash.with_indifferent_access
  end

  ❌ This fails
  it 'fails after transforming symbolized keys within a HashWithIndifferentAccess' do
    string_key_hash = {'a' => ->{{b: 1}}}.with_indifferent_access # Note this is a HashWithIndifferentAccess before transforming values
    string_key_hash.transform_values!(&:call)
    symbol_key_hash = {a: {b: 1}}
    # string_key_hash.with_indifferent_access is just string_key_hash.dup, meaning the symoblized keys
    # emitted by the transform_values! call are not converted to strings.
    expect(string_key_hash.with_indifferent_access).to eq symbol_key_hash.with_indifferent_access
  end
end

The Solution

OK, so now we know what's causing our false negative test results with .have_exact_props. There's a lot of ways to solve it, but they all end up being a version of "make sure you are comparing apples to apples".

I didn't like that my original solution had .with_indifferent_access access scattered around the internal code. I also think that, as evidenced two times now, being indifferent to the key type is a bit of a footgun.

The change here converts our props into hashes-with-symbolized-keys in a single place: the .computed_props method in the renderer, right before we merge the props together. Now the logic is in one place, and a developer can freely mix string and symbol keys in their controllers. InertiaRails will reconcile everything without duplicated keys. The RSpec matchers are also updated to use symbolized keys in order to match the output of .computed_props. Again, it won't matter whether a test uses strings or symbol keys or mixes them. InertiaRails treats them as equivalent.

I believe this is a much cleaner implementation with a smaller chance of unintended consequences.

That said, are there any other footguns from symbolizing keys that I might be missing? This change is tested, and all previous tests have passed, but this and the issues that prompted 3.1.1 both slipped through holes in the test suite.

[bknoles]

This is just a formatting change. I just like reading this on multiple lines better.

[bknoles]

This is taken directly from the bug report, with one tweak to ensure complete ambivalence towards the key type.

[BrandonShar]

Looks like the explanation was more work than the code 😄

Excellent description and the change makes good sense to me!

[bknoles]

Haha, it actually did take awhile to figure out which 5 lines needed to be changed but yea, the description might be overkill. I just found it interesting and wanted to share.