Lifecycle events not being issued correctly

[jonathan-s]

Bug Report

This issue is half feature proposal, half bug report.

Describe the bug

Assume that we have the following code, and when clicking on the element we issue a full page morph.

<td
  data-reflex="click->CalendarReflex#new_calendar_event"
  id="Day{{ date|date:'Ymd' }}"
  data-date='{{ date|date:'c' }}'>
</td>

When you click on that element the standard stimulus reflex controller will pass everything correctly to the backend. However none of the lifecycle methods will work.

To get the beforeNewCalendarEvent working correctly you would need to add `data-controller="calendar" which points to the controller that contains the method.

If have the data-controller included afterNewCalendarEvent will work the first time, but will stop working if you continue clicking. The reason for this is that element.reflexData for some reason no longer contains any data.

The proposal is that you should never need to include data-controller for lifecycle methods to work. If that is a must and you for some reason forgot to include data-controller lifecycle methods will fail silently which is no bueno.

Expected / desired behavior

Given that we know the method we are looking for (in this case afterNewCalendarEvent as an example. We should be able to find that method among the registered controllers and then execute that.

Assuming that there are two methods with the same name we would issue a warning that the lifecycle method won't be executed because we couldn't identify it properly.

(which may be problematic if you work with inheritance).

Additional info

This is the data that is being sent to front-end.

{
    "identifier": "{\\"channel\\":\\"StimulusReflex::Channel\\"}",
    "type": "message",
    "cableReady": true,
    "operations": {
        "morph": [
            {
                "selector": "body",
                "html": "[..snip...]",
                "childrenOnly": true,
                "permanentAttributeName": "data-reflex-permanent",
                "stimulusReflex": {
                    "target": "CalendarReflex#new_calendar_event",
                    "args": [],
                    "url": "http://localhost:8000/calendar/",
                    "attrs": {
                        "class": "fri",
                        "data-reflex": "click->CalendarReflex#new_calendar_event",
                        // data-controller is stimulus-reflex if data-controller is not defined
                        "data-controller": "calendar",
                        "id": "Day20201204",
                        "data-add-class-id": "backdrop",
                        "data-add-class-classes": "modal-backdrop fade show",
                        "data-date": "2020-12-04",
                        "data-action": "click->calendar#__perform",
                        "checked": false,
                        "selected": false,
                        "tagName": "TD"
                    },
                    "dataset": {
                        "data-reflex": "click->CalendarReflex#new_calendar_event",
                        // data-controller is stimulus-reflex if data-controller is not defined
                        "data-controller": "calendar",
                        "data-add-class-id": "backdrop",
                        "data-add-class-classes": "modal-backdrop fade show",
                        "data-date": "2020-12-04",
                        "data-action": "click->calendar#__perform"
                    },
                    "selectors": [],
                    "reflexId": "286a45a6-1d84-43b2-8c35-80c85040e511",
                    "resolveLate": false,
                    "xpath": "//*[@id=\'Day20201204\']",
                    "cXpath": "//*[@id=\'Day20201204\']",
                    // reflexController is stimulus-reflex if data-controller is not defined
                    "reflexController": "calendar",
                    "permanentAttributeName": "data-reflex-permanent",
                    "formData": "",
                    "identifier": "{\\"channel\\":\\"StimulusReflex::Channel\\"}"
                }
            }
        ]
    }
}

Versions

StimulusReflex

• Node package: 3.4.0

External tools

Django

Browser

Firefox / Chorme.

[leastbad]

The only way reflex-specific lifecycle methods are intended to work is based on you attaching a data-controller="foo" to the element.

What's of concern to me is the element.reflexData not containing data part. Are you saying it's undefined?

Can you please prepare an MVCE using https://github.com/leastbad/stimulus_reflex_harness so we can see what you're seeing?

[jonathan-s]

I was unable to replicate that in the rails application above. However, I've made some progress in terms of understanding what's happening. If someone would like to take a look I have a django app where I can replicate this.

We always have the data here:

https://github.com/hopsoft/stimulus_reflex/blob/master/javascript/stimulus_reflex.js#L103

Then at a later point we sometimes are not able to access element.reflexData right here:

https://github.com/hopsoft/stimulus_reflex/blob/master/javascript/stimulus_reflex.js#L496

[jonathan-s]

@marcoroth Since you changed this line here: https://github.com/hopsoft/stimulus_reflex/blob/6786181658ee3504fe19a5c496e1c2398a74368b/javascript/lifecycle.js#L18

Is there any reason why we don't do !reflexes[reflexId] instead?

If element.reflexData is empty as in my case (which I frankly still don't understand why that is), it would be possible to fill in the reflex data again on the next line with

element.reflexData = reflexes[reflexId].promise.data

[leastbad]

Is this still a thing, @jonathan-s ?

[jonathan-s]

@leastbad No longer a thing so closing this. I mentioned the following in the PR, do you have any thoughts around that?

On a related note, I still think it would be nice to not need to mention data-controller. Another idea for this would be to have it working by naming convention, if you have a CalendarReflex it would try to find a calendar as a controller and execute the lifecycle methods there.

[leastbad]

This has come up a few times. I understand why you suggest it.

It's one thing to put our own stimulus-reflex controller on an element. I think it's a step further / too far to assume that we want to put their controller on the element.

If we'd started this early on, we might well have ended up with a different approach.

One thing you could propose is the idea of a naming scheme where if you specify data-reflex="foo" that it might search for and potentially attach a foo_reflex_controller.js controller. That actually seems pretty reasonable, but that's no guarantee @hopsoft and the others would agree.