Is controller.scope.element safe to use?

Wish there was a bit more to the docs, but for now…

I want to make sure accessing the scope for the controller context is safe to use. I’m using it now in connect() for adding event listeners to the element and want to make sure this is safe and not an API that would readily change.

Use case:
Building out reusable components for our Rails applications but this seems redundant:

<table data-controller="indexTable" data-target="indexTable.table">

When this should be enough

<table data-controller="indexTable">

And adding event listeners on each row seem unnecessary:

<tr data-href="<%= account_path(account) %>" data-action="click->indexTable#show">
  ...
</tr>

You can use this.element in a controller instance.

That’s the preferred approach, and you’re missing out on much of Stimulus’s benefit if you don’t use it. Why does it seem “unnecessary,” and why does manually adding and removing event listeners in the controller seem more appropriate to you?

Can’t speak for @pelted, but it’s a small annoyance to repeat the same data-actions and data-targets across elements that share the same controller.

For example, I have an infinite scrolling controller that can be added to any page. Instead of adding data-action=“scroll@window->infinite-scroll#didScroll” to each element, I just add the event listener in connect().

But this is really small beans, and I agree that we’re almost always better off expressing events and targets in the DOM rather than in the controller. Thankfully Stimulus is flexible and lets me choose what makes the most sense in each case.

Isn’t it better to attach the listener to the parent and get the clicked element (including all the data attributes) using the target property? You only create (and destroy if necessary) 1 listener, once.

What benefit does Stimulus offer by creating an event listener for each <tr>?

Exactly. Which is why I’m attaching to the table and not the rows. My original example had sort of an anti-pattern at the end. I guess adding a single data-action="click-> on the table would work, but I found that debugging the lifecycle of the event listeners to be difficult so I’m adding one to the table in connect, and removing it manually in disconnect and also in my event handler function.

I guess the original question is answered though. I did not notice this.element, and digging into scope to get it smelled a bit.

That’s exactly what Stimulus does for you with data-actions.

What aspect of the lifecycle were you trying to debug and what was difficult?

The issue I was trying to debug related to having several data-actions (one per tr). It seemed like the event listeners were sticking around when I think they should have been removed. (I was probably wrong)

There were a couple of different use cases, one where once an action on a tr was complete I no longer wanted to be listening on that row so the listener needed to be removed manually.

The other case was standard click the row to move to the show route. I think the listeners I was seeing sticking around may have been a result of Turbolinks cache but I was already down the hole of manually adding and removing so I didn’t dig in further.

After refactoring the JS I believe data-actions is doing exactly what it should be doing and I should embrace it.

If you have hundreds or thousands of DOM nodes then yes, it might be worth optimizing. The tradeoff is that the event target may not be the element you’re interested in. Consider the following HTML:

<table data-action="click->results#removeRow">
  <tr>
    <td>
      <button>Remove</button>
    </td>
  </tr>
</table>

When clicked, event.target will be <button> and event.currentTarget will be <table>. So you’d need to traverse upward from the target to find the <tr>.

<table>
  <tr data-action="click->results#removeRow">
    <td>
      <button>Remove</button>
    </td>
  </tr>
</table>

In this example event.currentTarget will be <tr>.

Exactly, but it is actually more complicated than that. My example is over simplified. You also have to call stopPropagation() if an anchor or even an inner i is the target. But with Turbolinks you need to make sure not to prevent it’s interception of the links.

I was originally just trying to present a simple case to make sure what I was doing was safe.

Right right, I came across that thing and had to do a little bit of DOM traversing. Like, e.target.closest('.some-class').

And about e.currentTarget, I had no idea it existed. Would love to see these small little gems in the docs. If you click on the <button> within the <tr>, does e.currentTarget still return the <tr>?

EDIT: ah nevermind, “event.currentTarget will be <table>” answers that.