Skip to content

Commit

Permalink
Fix ReflexData parsing (#688)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Matt-Yorkley authored Mar 6, 2024
1 parent 5325da4 commit 078db30
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 27 deletions.
2 changes: 1 addition & 1 deletion lib/stimulus_reflex/reflex_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class StimulusReflex::ReflexData
attr_reader :data

def initialize(data)
@data = data.deep_transform_keys { |k| k.to_s.underscore }.with_indifferent_access
@data = data.deep_merge(data.deep_transform_keys { |k| k.to_s.underscore }).with_indifferent_access
end

def reflex_name
Expand Down
54 changes: 28 additions & 26 deletions test/element_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require_relative "test_helper"

class StimulusReflex::ElementTest < ActiveSupport::TestCase
element = StimulusReflex::Element.new({
reflex_data = StimulusReflex::ReflexData.new({
"attrs" => {
"user" => "First User",
"user-id" => "1"
Expand All @@ -17,6 +17,8 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
}
})

element = StimulusReflex::Element.new(reflex_data.data)

test "should be able to access attributes on element itself" do
assert_equal "First User", element.user
assert_equal "First User", element["user"]
Expand Down Expand Up @@ -72,7 +74,7 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should pluralize keys from datasetAll" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-reflex" => "click",
Expand All @@ -84,9 +86,9 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
"data-name" => ["steve", "bill", "steve", "mike"]
}
}
}
})

dataset_all_element = StimulusReflex::Element.new(data)
dataset_all_element = StimulusReflex::Element.new(reflex_data.data)

assert_equal "click", dataset_all_element.dataset.reflex
assert_equal "male", dataset_all_element.dataset.sex
Expand All @@ -97,7 +99,7 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should pluralize irregular words from datasetAll" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {},
"datasetAll" => {
Expand All @@ -110,9 +112,9 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
"data-mouse" => ["mouse"]
}
}
}
})

pluralize_element = StimulusReflex::Element.new(data)
pluralize_element = StimulusReflex::Element.new(reflex_data.data)

assert_equal ["cat"], pluralize_element.dataset.cats
assert_equal ["child"], pluralize_element.dataset.children
Expand All @@ -124,32 +126,32 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should not pluralize plural key" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"datasetAll" => {
"data-ids" => ["1", "2"]
}
}
}
})

assert_equal ["1", "2"], StimulusReflex::Element.new(data).dataset.ids
assert_nil StimulusReflex::Element.new(data).dataset.idss
assert_equal ["1", "2"], StimulusReflex::Element.new(reflex_data.data).dataset.ids
assert_nil StimulusReflex::Element.new(reflex_data.data).dataset.idss
end

test "should not build array with pluralized key" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-ids" => "1"
}
}
}
})

assert_equal "1", StimulusReflex::Element.new(data).dataset.ids
assert_equal "1", StimulusReflex::Element.new(reflex_data.data).dataset.ids
end

test "should handle overlapping singluar and plural key names" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-id" => "1",
Expand All @@ -165,9 +167,9 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
"data-duplicate-value" => ["20", "21", "22"]
}
}
}
})

overlapping_keys_element = StimulusReflex::Element.new(data)
overlapping_keys_element = StimulusReflex::Element.new(reflex_data.data)

assert_equal "1", overlapping_keys_element.dataset.id
assert_equal ["2", "3", "4"], overlapping_keys_element.dataset.ids
Expand All @@ -180,7 +182,7 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should return true for boolean data attributes" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-short" => "t",
Expand All @@ -189,9 +191,9 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
"data-empty" => ""
}
}
}
})

element_with_boolean_attributes = StimulusReflex::Element.new(data)
element_with_boolean_attributes = StimulusReflex::Element.new(reflex_data.data)

assert element_with_boolean_attributes.boolean[:short]
assert element_with_boolean_attributes.boolean[:long]
Expand All @@ -205,17 +207,17 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should return false for falsey data attributes" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-short" => "f",
"data-long" => "false",
"data-num" => "0"
}
}
}
})

element_with_falsey_attributes = StimulusReflex::Element.new(data)
element_with_falsey_attributes = StimulusReflex::Element.new(reflex_data.data)

refute element_with_falsey_attributes.boolean[:short]
refute element_with_falsey_attributes.boolean[:long]
Expand All @@ -227,17 +229,17 @@ class StimulusReflex::ElementTest < ActiveSupport::TestCase
end

test "should return numeric values" do
data = {
reflex_data = StimulusReflex::ReflexData.new({
"dataset" => {
"dataset" => {
"data-int" => "123",
"data-float" => "123.456",
"data-string" => "asdf"
}
}
}
})

element_with_numeric_attributes = StimulusReflex::Element.new(data)
element_with_numeric_attributes = StimulusReflex::Element.new(reflex_data.data)

assert_equal 123.0, element_with_numeric_attributes.numeric[:int]
assert_equal 123.456, element_with_numeric_attributes.numeric[:float]
Expand Down

0 comments on commit 078db30

Please sign in to comment.