search

stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.29k stars 174 forks source link

Fix ReflexData parsing #688

Closed Matt-Yorkley closed 8 months ago

Matt-Yorkley commented 9 months ago

Bug Fix

Description

The data in StimulusReflex::ReflexData was recently changed to transform all keys to underscored keys without preserving the original kebab-case keys (like data-controller), but this causes breaking changes elsewhere in the codebase. Both sets of keys need to be preserved but this was missed by the relevant tests because the test input was using a plain hash instead of instantiating a ReflexData object (see updated test setup).

Why should this be added

The previous change caused issues like missing attributes in element.dataset in Reflexes. It's in main, but it's not released yet.

Checklist

netlify[bot] commented 9 months ago

Deploy Preview for stimulusreflex ready!

Name Link
Latest commit 6b6453d9b42aed46312ef61b7b19be7aea8d0124
Latest deploy log https://app.netlify.com/sites/stimulusreflex/deploys/65db8ae7dbc68f0008151db6
Deploy Preview https://deploy-preview-688--stimulusreflex.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Matt-Yorkley commented 8 months ago

I had the same thought, a couple of full-stack tests would be great. Maybe a dummy app and some System Tests with Capybara/Cuprite is the way to go?