Don't choose first controller if no matching StimulusReflex-enabled controller was found

[marcoroth]

Type of PR

Bug Fix

Description

This pull request solves an edge-case for scenarios when StimulusReflex tries to detect the right controller to use for performing the StimulusReflex __perform controller action for executing reflexes.

For example, if an element had a data-controller and a data-reflex attribute, and the controller in data-controller was a StimulusReflex-enabled controller, but didn't match the Reflex specified in the data-reflex there were some cases where it wouldn't add the default stimulus-reflex controller to the element, but would still reference the stimulus-reflex controller in the data-action attribute. This basically lead to a no-op for the reflex action.

Like:

<a
  data-controller="
    some-sr-controller
  "
  data-reflex="click->Example#doSomething"
></a>

was expanded to this in some cases:

<a
  data-controller="
    some-sr-controller
  "
  data-reflex="click->Example#doSomething"
+ data-action="click->stimulus-reflex#__perform"
></a>

Whereas it should have been:

<a
  data-controller="
    some-sr-controller
+   stimulus-reflex
  "
  data-reflex="click->Example#doSomething"
+ data-action="click->stimulus-reflex#__perform"
></a>

With this, the data-action is not referencing an undefined Stimulus controller anymore and the reflex should execute again.

Why should this be added

For consitancy reasons, so that the declarative reflex syntax still continues to make sense and work as expected.

Maybe this is even fixing @Laykou's issue in #659 which I couldn't reproduce.

Thanks to @foliosus for helping to debug this!

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	c4f1252
🔍 Latest deploy log	https://app.netlify.com/sites/stimulusreflex/deploys/64b1678e75f9110009a9914f
😎 Deploy Preview	https://deploy-preview-670--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 configuration.

[Laykou]

@marcoroth Shouldn't it be expanding into data-action="click->some-sr-controller#__perform"?

[marcoroth]

@Laykou it could, but I feel like it's weird if it "leaks" the lifecycle callbacks into a controller which potentially is not related to the reflex itself.

But maybe it should?

The issue could also be resolved if we do this instead, yeah:

<a
  data-controller="
    some-sr-controller
  "
  data-reflex="click->Example#doSomething"
></a>

to this

<a
  data-controller="
    some-sr-controller
  "
  data-reflex="click->Example#doSomething"
+ data-action="click->some-sr-controller#__perform"
></a>

[Laykou]

How would this work then? https://docs.stimulusreflex.com/guide/lifecycle.html#callback-methods

having

<div data-controller="example">
  <a href="#" data-reflex="Example#masticate">Eat</a>
</div>

I believe it must expand to

<div data-controller="example">
  <a href="#" data-reflex="Example#masticate" data-action="click->example#__perform">Eat</a>
</div>

in order to calll callbacks in example_controller.js, right?

import ApplicationController from './application_controller.js'

export default class extends ApplicationController {
  beforeReflex(anchorElement) {
    const { reflex } = anchorElement.dataset
    if (reflex.match(/masticate$/)) anchorElement.innerText = 'Eating...'
    if (reflex.match(/defecate$/)) anchorElement.innerText = 'Pooping...'
  }
}

Otherwise how would this beforeReflex from example_controlller.js be called if it expands only to generic data-action="click->stimulus-reflex#__perform" ?

[marcoroth]

Yeah, this is currently how it works. It will also look on any ancestor to find a matching StimulusReflex-enabled controller.

<div data-controller="example">
  <a href="#" data-reflex="click->Example#masticate" data-action="click->example#__perform">Eat</a>
</div>

Only if the reflex specified in data-reflex doesn't match a StimulusReflex-enabled controller it will add the stimulus-reflex controller.

So this:

<div data-controller="something-else">
  <a href="#" data-reflex="click->Example#masticate">Eat</a>
</div>

would expand into:

<div data-controller="something-else">
  <a href="#" data-reflex="click->Example#masticate" data-controller="stimulus-reflex" data-action="click->stimulus-reflex#__perform">Eat</a>
</div>

[mbajur]

Just for the record - this PR also solved a case in which i had a stimulus_reflex button with plain stimulus controller attached to it and thanks to that fact - added stimulus-reflex actions were not triggered because stimulus-reflex controller was not being attached to the element alongside with the original controller.

[Laykou]

@marcoroth May I get back to this - is it possible to override the default stimulus-reflex e.g. when I want to use my-custom-base-stimulus-reflex controller instead?

I'd like to setup som default beforeReflex and afterReflex actions (like always adding spinner, if element is a button, etc...) and it would help a lot to be able to define this globally

Is it possible to:

• either override the default stimulus-reflex
• or somehow initialize the StimulusReflexController with "global" custom lifecycle callbacks ?