Don't add data-controller to reflex element if parent already holds instance

[marcoroth]

Type of PR

Bug fix

Description

There's was an edge case where we would add another data-controller attribute to the reflex element even if a parent element already had a matching controller instance.

This markup:

<div data-controller="example">
  <button 
    data-reflex="click->Example#increment"
  >Click</button>
</div>

would get transformed to:

<div data-controller="example">
  <button 
    data-reflex="click->Example#increment" 
+   data-controller="example" 
+   data-action="click->example#__perform"
  >Click</button>
</div>

Which ends up with two controller instances of example even if we actually just needed one.

This pull request optimizes this by just conditionally adding the example controller to the data-controller attribute on the reflex element.

Now with this pull request the end-result is just:

<div data-controller="example">
  <button 
    data-reflex="click->Example#increment" 
+   data-action="click->example#__perform"
  >Click</button>
</div>

Why should this be added

We shouldn't create more controller instances than necessary. In this case it was apparent because the connect() callback was fired twice. To not fire the method twice we add the condition.

Checklist

• My code follows the style guidelines of this project
• Checks (StandardRB & Prettier-Standard) are passing

[netlify]

✅ Deploy Preview for stimulusreflex ready!

Name	Link
🔨 Latest commit	690f78f
🔍 Latest deploy log	https://app.netlify.com/sites/stimulusreflex/deploys/640f79af7a9207000822b2a0
😎 Deploy Preview	https://deploy-preview-636--stimulusreflex.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[arambert]

I want to emphasize the importance of this PR, indeed if we want to add some Stimulus controls (on connect, targets, actions on elements, etc.) the fact that the component controller is duplicated on each reflex element creates weird errors ans makes it almost impossible. The only sane solution is be to create a second stimulus controller, which feels wrong to me.

I'd really like to be able to have only one controller!

[arambert]

Hi,

Are you planning to finalize & merge this PR?
Do you need any help?

Thanks

[marcoroth]

Hey @arambert, yeah I wanted to get this merged soon! I've been exploring some other edge cases, which I wanted to address in the pull request. But either way, I'm going to finish this one as-is!

Thanks for your patience! 🙏🏼