Pass data attributes as action method argument

[adrienpoly]

Very often the question comes up on how to pass arguments to Stimulus action methods.

Global parameters can be passed to the action using the data API but when a controller has multiple elements and individual parameters (an id by example) for each of those elements, it is a common practice to pass the value using the standard dataset API: event.currentTarget.dataset.id

One typical example would be for a set of buttons with ids and associated fetch actions

<div data-controller="items" data-items-base-url="https://api.stimulus.org/upvote/">
  <button data-action="items#upvote" data-id='1'></button>
  <button data-action="items#upvote" data-id='2'></button>
  <button data-action="items#upvote" data-id='3'></button>
</div>

...
upvote(e) {
  const id = e.currentTarget.dataset.id
  fetch(this.baseUrl + id, { method: "POST" })
}

get baseUrl() {
  return this.data.get("baseUrl")
}
...

Proposal

This PR is a draft proposal for a more concise API by passing all data attributes value of the currentTarget as a parameter of the action.

action(event, data) {}

Using basic destructuring the above example can be simplified like this:

...
upvote(e, { id }) {
  fetch(this.baseUrl + id, { method: "POST" })
}
...

I think this could make it simpler to understand and avoids confusions around target and currentTarget.
If the element is of a type HTMLFormElement we could also pass the value at the same time.

Just wanted to propose this for now if you feel this could make sense, I ll add some associated test

[tpetry]

1. Wouldn't passing a dataset object like this.data of controllers fit much more to the stimulus way?
2. If there are multiple actions for an element the data values may interfere. I think it would be best to prefix them with the controller and action, so data-id would be data-items-click-upvote-id.

@jaan @sstephenson Whats your opinion on the pull request? It's small but a very usable improvement

[dhh]

I'd like to see a complete params concept encapsulate this. So:

<div data-controller="items" data-items-base-url="https://api.stimulus.org/upvote/">
  <button data-action="items#upvote" data-items-upvote-id-param='1'></button>
  <button data-action="items#upvote" data-items-upvote-id-param='2'></button>
  <button data-action="items#upvote" data-items-upvote-id-param='3'></button>
</div>

// Call signature is event, params
upvote(e, { id }) {
  fetch(this.baseUrl + id, { method: "POST" })
}

This way params are separated from any other type of data-* on the action element, they're namespaced so different actions can have different params, and we have a concept that's encapsulated fully.

[dhh]

Actually, I'd rather still even just make the event part of the params, so you can chose whether or not to destruct it:

// Doesn't need the event
upvote({ id }) {
  fetch(this.baseUrl + id, { method: "POST" })
}

// Does need it event
upvote({ id, event }) {
  fetch(this.baseUrl + id, { method: "POST" })
  event.preventDefault()
}

[tpetry]

This would break backwards compatibility

[adrienpoly]

I'd like to see a complete params concept encapsulate this. So:

<div data-controller="items" data-items-base-url="https://api.stimulus.org/upvote/">
  <button data-action="items#upvote" data-items-upvote-id-param='1'></button>
  <button data-action="items#upvote" data-items-upvote-id-param='2'></button>
  <button data-action="items#upvote" data-items-upvote-id-param='3'></button>
</div>

This way params are separated from any other type of data-* on the action element, they're namespaced so different actions can have different params, and we have a concept that's encapsulated fully.

I started implementing and testing with this namespacing approach. I agree that it does fully encapsulate the params but it could become very verbose to write if we have several controllers needing the same parameter

This same example with the id become very long to write.

 <div data-controller="controller1 controller2" data-items-base-url="https://api.stimulus.org/upvote/">
   <button data-action="controller1#upvote controller2#doSomething" 
                 data-controller1-upvote-id-param='1' 
                 data-controller2-upvote-id-param='1'></button>
   <button data-action="controller1#upvote controller2#doSomething" 
                  data-controller1-upvote-id-param='2' 
                  data-controller2-upvote-id-param='2'></button>
 </div>

The js part is simplified but the html becomes much more complex to write.

I think that the type of parameters that we need to pass to the functions are unique as they are linked to the caller element of the action. That was my idea behind this for not using a namespace.

I like the suggestion of adding the param prefix. I think it would make it easier to port the value api #202 to this feature with something like:

export default class extends Controller {
  static params = {
    id: Number,
    active: Boolean
  }
 ...

  upvote(e, { id, active }) {
    //active is a Boolean
    if (active) { 
      fetch(this.baseUrl + id, { method: "POST" })
    }
  }
}

[dhh]

Don't have a problem bumping a major version for this, and we can have a feature toggle that still supports the old signature.

[dhh]

@javan Has an idea for this that doesn't involve breaking backwards compatibility. I'll let him present that.

[javan]

I'm not a fan of moving event into an object because it makes the most common usage like calling event.preventDefault() more verbose. You'd have to either destructure the event out or access a nested params.event property. And it's breaking change.

Another option, which wouldn't be a breaking a change, is to hang params off the event object:

<button data-action="items#upvote" data-items-upvote-url-param="/items/1/votes">…</button>

upvote(event) {
  event.preventDefault()
  console.log(event.params) // { url: "/items/1/votes" }
}

Or, if you're only interested in the params:

upvote({ params }) {
  console.log(params) // { url: "/items/1/votes" }
}

Or even:

upvote({ params: { url } }) {
  console.log(url) // "/items/1/votes"
}

[dhh]

I also do like the idea of defining the params ala values, btw. Rather than having them be fully namespaced in the attributes. Would give us the power to typecast as well.

The only issue is that it's a global params definition, even if the invocation is mostly per-action. But maybe that doesn't matter. A lot of the time there's bound to be overlap anyway (like with ids).

[javan]

Because params would be read-only (unlike values which are read/write), we could (attempt to) typecast them automatically without the need to declare types in the controller. Example:

function getParams(element, descriptor) {
  const params = {}
  const [ identifier, action ] = descriptor.split("#")
  const pattern = new RegExp(`^data-${identifier}-${action}-(.+)-param$`)
  for (const { name, value } of element.attributes) {
    const match = name.match(pattern)
    const key = match && match[1]
    if (key) params[key] = typecast(value)
  }
  return params
}

function typecast(value) {
  try {
    return JSON.parse(value)
  } catch (o_O) {
    return value
  }
}

<button data-action="item#upvote" 
  data-item-upvote-id-param="12345" 
  data-item-upvote-url-param="/votes" 
  data-item-upvote-active-param="true">…</button>

❭ getParams(button, "item#upvote")
{ id: 12345, url: "/votes", active: true }

[dhh]

I like that. The primary appeal for declaring the params up front would be a sense that you'd say "this controller uses these params", especially since you'd feel more OK with having the params listed without controller/action namespaces. But starting to to think that it probably doesn't matter. We could even do both.

So the default would be that params are simply available to all actions, but you could also namespace them, in case there was a conflict between two equally named params for the same element. So:

<button data-action="item#upvote" 
  data-id-param="12345" 
  data-item-upvote-url-param="/votes" 
  data-item-upvote-active-param="true">…</button>

In that case, params.id would be available to all controllers/actions invoked off that element. But params.url and params.active would only be there for the item#upvote. I think this works particularly well since params is now just a event.params call, that you can destruct as you please on the method signature.

[saiqulhaq]

please merge this PR
currently not easy to pass argument

I think what @dhh suggested is very good

[jmas]

@saiqulhaq what about:

<button type="button" data-action="controller.doAction" data-id="1">Do Action</button>

controller:

doAction(event) {
    console.log('data attached to button=', event.target.dataset);
    console.log('id=', event.target.dataset.id);
}

or:

doAction({ target: { dataset: { id } } }) {
    console.log('id=', id);
}

[saiqulhaq]

@jmas data-id is not very clear for me
if we use data-id-param, then it means it's a parameter for stimulus's action.
and also we can add params property to event object

so it could be

doAction({ params }) {
    console.log('id=', params.id);
}

[adrienpoly]

@saiqulhaq I haven't had time yet to update this PR as per our talk above. I plan to do so by the end of the month.

[chrislabarge]

I love the idea of global and controller/action specific params. This would allow me to open the scope of a lot of my stimulus controllers.

[rubab]

How can I access data params in controller action?

[saiqulhaq]

@rubab you can use $(event.target).data(' data attribute here ')

[scarroll32]

This looks great @adrienpoly will it be merged?

[jturmel]

Is this PR still in the works?

[adrienpoly]

@jturmel yeah still need to work on it. Last time I hit a few limits of my TS knowledge but I have practiced a bit more TS since then. I ll try to get back to it in August but if you feel like taking over feel free to give it a try

[dancallaghan]

I see a possible issue with having shared params. What constitutes a shared param vs. a namespaced param? Would it be a case of looking through each of the actions, finding their namespaced params and the remainder are shared?

In this example:

 <button data-action="item#upvote remote#load"
  data-id-param="12345"
  data-remote-load-url-param="/votes/summary"
  data-item-upvote-url-param="/votes">…</button>

Would remoteLoadUrl be passed to item#upvote? Or would it be excluded because it belongs to another action's namespace?

Maybe we could get around the ambiguity by using a JSON syntax? Seems a little messier though…

<button data-action="remote#load item#upvote"
  data-params="{ id: 12345 }"
  data-remote-load-params="{ url: '/votes/summary' }"
  data-item-upvote-params="{ url: '/votes', active: true }">…</button>

Another option would be to have a reserved global param namespace. Something like shared/global

 <button data-action="item#upvote remote#load"
  data-shared-id-param="12345"
  data-remote-load-url-param="/votes/summary"
  data-item-upvote-url-param="/votes">…</button>

[adrienpoly]

I have noticed a few things that needs to be corrected after the merge with 2.0 will try to address them

[adrienpoly]

I have corrected a few merge issues with 2.0 and updated a bit the code to match as close as possible to the latest additions.

The features remain the same as described in my previous post

As always let me know if you have questions / suggestions / comments

[sagarrishabh]

Merge this bro, I need it. 😄

[leehericks]

@adrienpoly thank you for your hard work!

I’ve been thinking about the API and documentation for this PR all day.

The targets, values, and css classes APIs all have clear, explicit declarations at the top of the controller.

Actions methods currently

1. are left empty upvote()
2. or reference the event object upvote(event)

The params API should similarly have clear declarations in the action method arguments.

I think the rest of the event object can still be accessed with this destructuring syntax:

upvote({ params: { id, url = “/test/me” }, ...event })

but this buries the event at the end of the arguments.

I’d like to suggest adding an event getter property to the event object (in addition to params):

get event() {
  return this;
}

which allows us the write the event at the front of the destructured list:

upvote({ event, params: {
  id = undefined, 
  url = undefined, 
  active = true } }) {
  ...
}

allowing the params to feel like an appended optional declaration:

1. upvote()
2. upvote(event)
3. upvote({ event, params: { id, url = “/fallback/path” } })

As a side note, inserting params into the event object still feels very awkward to me. Does this not feel cleaner?

upvote(event, params) {
  const {
    id = undefined, 
    url = undefined, 
    active = true 
  } = params
  ...
}

[dancallaghan]

I feel like destructuring too much in the function declaration reduces readability (agreeing with your side note). Why not something like this?

upvote(event) {
  const {
    id = undefined, 
    url = undefined, 
    active = true 
  } = event.params
  // ...
}

[leehericks]

@dancallaghan Indeed, this is much clearer, but it still irks me that the params are riding in the event object and we can’t require developers to declare params like we do for targets, values and css classes. Then again, technically the developers could just access the data bypassing those other APIs, so the final solution needs to offer enough benefit that everyone wants to use it.

Are we naming this feature correctly if they don’t appear in the method parameters? Are we not really just referring to a subset of data values declared on the element with the action descriptor?

[leehericks]

Then again, we can simply add a section to the end of Event Objects in the Actions reference entitled “Event Params”, where we can show the best practice example of declaring the event params.

so, this covers getters and @javan said here that these params are read-only so no need for setters.

I don’t remember seeing discussion above about adding existential properties, which could be a useful check that lets the developer know, for example, that the attribute wasn’t in the data (or is undefined?) so the restructured value is the provided default value.

If we go with the “Event Params” branding then what about event.hasIdParam which checks if the param is defined in the data tags?

While we may not want to overly pollute the event object, event.params.hasIdParam feels redundant.

Also event.params.hasId does not match with the targets, values and css classes APIs.

[seb-jean]

Hi,
I tried this new feature and I have always undefined value when I do :

upVote({params}) {
    console.log(params)
}

[adrienpoly]

@seb-jean are you running the hotwired/stimulus 3 beta 2 version ?

[panoply]

Same issue as @seb-jean in v3.0.1 using the @hotwired/stimulus module. Params are simply not passed.

Looking at the source for the actions logic, the re-assignment of the event target is a bit of code smell imo and just to pass the params object? Also, why is the params logic wrapped in a getter, deconstructed and finally re-assigned? Just feels so extraneous, I get the approach but it's definitely not ideal.