[StimulusBundle] feat: normalize parameters names given to twig helper 'stimulus_action'

[norival]

Q	A
Bug fix?	no
New feature?	yes
Tickets	NA
License	MIT

This allows giving a parameter name as 'myParam', and it will be printed as 'data-controller-name-my-param-param', which will be interpreted as 'myParam' by the Stimulus controller.

[smnandre]

Nice catch @norival !

@weaverryan : Is it seen as a bugfix.. or a new feature ? In the later, BC break alert :)

[weaverryan]

This allows giving a parameter name as 'myParam', and it will be printed as 'data-controller-name-my-param-param', which will be interpreted as 'myParam' by the Stimulus controller.

What would the myParam have data-controller... attribute looked like before this PR? I'm also interested if there is any possibility that changing from the old string to the new string could potentially break someone's app... or if the old attribute was totally wrong and unusable, and now this fixes it.

[smnandre]

Well after playing with this a bit, i'm 100% to consider this a bugfix.

And it's more a "HTML/DOM/JS" problem than a specific Stimulus one.

<div data-foo-bar="foo-bar" data-fooBar="fooBar">...</div>
<div data-fooBar="fooBar" data-foo-bar="foo-bar">...</div>

Data attributes are case-incensitive (and parsed in lowercase), and the snake to camelCase conversion between HTML and JS is the expected behaviour.

Big Crocodile

Given a parameter named bigCrocodile

Before

Before the fix made by @norival , the HTML code rendered by the function was

<button 
    data-action="foo#bar" 
    data-foo-bigCrocodile-param="green" 
  >...</button>

But in the Stimulus controller, the parameters passed were in lowercase

{bigcrocodile: "green"}

After

After the fix, the HTML code will be

<button 
    data-action="foo#bar" 
    data-foo-big-crocodile-param="green" 
  >...</button>

and the parameters passed to the action

{bigCrocodile: "green"}

That also means that is someone called the stimulusAction with snake-case parameters, nothing changes (they still get the parameter in camelCase)

[norival]

Great analysis @smnandre ! 😃

Actually, I think there is a case where it would break things.

If you declare the parameters in the twig function like this:

{{ stimulus_action('controller', 'action', 'click', {bigCrocodile: "green"}) }}

this will render ...data-controller-bigcrocodile-param="green" and register it as event.params.bigcrocodile. After the 'fix', it would render ...data-controller-big-crocodile-param="green" and register it as event.params.bigCrocodile. So this would break the controller code using this parameter.

[smnandre]

this will render ...data-controller-bigcrocodile-param="green"

I don't find where this transformation is done (bigCrocodile -> bigcrocodile in HTML) ?

[norival]

I don't find where this transformation is done (bigCrocodile -> bigcrocodile in HTML) ?

My bad, the HTML is indeed data-controller-bigCrocodile-param="green", but in the DOM, it is interpreted as data-controller-bigcrocodile-param="green", and, when getting the dataset from JS, it is {controllerBigcrocodileParam: "green"} (in Firefox and Chrome, at least).

[smnandre]

I checked, and Stimulus handles it (on macOs / Firefox + Chrome) as {bigcrocodile: green} indeed. So there is a difference.

But... if a developer passed "bigCrocodile" in HTML and then manually used "bigcrocodile" in the JS code.... they should have made the PR you just wrote instead :))

[norival]

But... if a developer passed "bigCrocodile" in HTML and then manually used "bigcrocodile" in the JS code.... they should have made the PR you just wrote instead :))

Haha, I agree, that would be a bit weird 😅

[weaverryan]

Can you update the CHANGELOG entry to make it a little longer and explain the behavior change if you were relying on the broken behavior?

[norival]

Can you update the CHANGELOG entry to make it a little longer and explain the behavior change if you were relying on the broken behavior?

@weaverryan done! Is it ok like that?

[weaverryan]

One last tweak :)

[weaverryan]

Break onto multiple lines when it gets long (around 72ish chars) and indent the other lines the same as the word Normalize. Also, slight rewording:

camelCase (eg. bigCrocodile) were incorrectly registered ...

And change the last sentence to:

This was fixed, which means they are now correctly registered as camelCase (event.params.bigCrocodile).

[smnandre]

And... long live the crocodile 😉

[norival]

Done 😃

[weaverryan]

Thank you Xavier!