pgaspar commented 3 years ago

Bug Report

Describe the bug

After a recent change in our codebase we started noticing a series of interesting errors in production:

NoMethodError: undefined method `cyan' for :"#foo-bar":Symbol

(#foo-bar is an example ID here). After some investigation, we found the issue came from a morph call where we pass in a Hash with symbol keys as the argument, as described by the Morphing Multiplicity section in the docs:

morph '#foo-bar': '<div>Some HTML</div>'

Changing the Hash key to be a String instead of a Symbol fixes the issue:

morph '#foo-bar' => '<div>Some HTML</div>'

The underlying problem seems to be that the Logger class does not coerce the selector to string, like it does for operation and a few other things.

Note: this issue doesn't seem to affect the CableReady broadcast itself, even if logging breaks. So things work as intended for the end-user.

Note 2: in development I don't actually see an error - it just doesn't log anything unless we change the keys to String or stop using Morphing Multiplicity. I didn't try to understand why we were not seeing an error, but it may be related to our own setup and not the StimulusReflex gem.

To Reproduce

Morph using Morphing Multiplicity with a Hash with Symbol keys (example above).

Expected behavior

SR is able to log the morph. Example:

1/2 MyReflex#do_something -> #foo-bar via Selector Morph (inner_html)

Versions

StimulusReflex

Gem: 3.4.1
Node package: 3.4.1

marcoroth commented 3 years ago

Hey @pgaspar, thanks for filing this detailed issue. ❤️

I can reproduce what you are describing. It seems like if we append a to_s at the end of the selector method that it then works as expected.

---    reflex.broadcaster.operations[current_operation - 1][0] 
+++    reflex.broadcaster.operations[current_operation - 1][0].to_s

Are you up to open a pull request to fix this?

leastbad commented 3 years ago

I have actually been working on this for a white, sorry I should have grabbed the ticket. 🤦

pgaspar commented 3 years ago

No problem! I'm glad this led to a deeper fix 😊 Thank you!