Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Using action option changes the order of actions #718

Open
janko opened this issue Aug 29, 2023 · 2 comments
Open

Using action option changes the order of actions #718

janko opened this issue Aug 29, 2023 · 2 comments

Comments

@janko
Copy link

janko commented Aug 29, 2023 •
edited

In certain situations, using an action option such as :prevent can change the order in which actions registered in the same data-action are executed.

Here is a self-contained example demonstrating the problem:

<html>
  <head>
    <script type="module">
      import { Application, Controller } from "https://unpkg.com/@hotwired/stimulus/dist/stimulus.js"
      window.Stimulus = Application.start()
      Stimulus.debug = true
      Stimulus.register("one", class extends Controller {
        foo() {}
      })
      Stimulus.register("two", class extends Controller {
        bar() {}
      })
    </script>
  </head>
  <body>
    <div data-controller="two">
      <div data-controller="one">
        <a href="#" data-action="one#foo two#bar">Click me</a>
      </div>
    </div>
  </body>
</html>

Without the action option, the debug output shows the correct order:

one #foo
two #bar

However, if I use an action option:

<a href="#" data-action="one#foo:prevent two#bar">Click me</a>

The debug output shows the order in which actions are executed has changed:

two #bar
one #foo

This seems like a bug to me. I encountered it in a scenario where the order of actions was important, and it took me a while to realize it's because of the action option.
@adrienpoly
Copy link
Member

Thanks @janko for this detailed report. I think this PR #684 might fix it.

The PR mentions action added dynamically so it is not exactly the same. I ll see if I can create a test reproducing your but and run it against that PR.

@adrienpoly
Copy link
Member

I ran some additional test

given this original test from the test suite:

async "test adding an action to the left"() {
    this.actionValue = "c#log3 c#log d#log2"
    await this.nextFrame
    await this.triggerEvent(this.buttonElement, "click")

    this.assertActions(
      { name: "log3", identifier: "c", eventType: "click", currentTarget: this.buttonElement },
      { name: "log", identifier: "c", eventType: "click", currentTarget: this.buttonElement },
      { name: "log2", identifier: "d", eventType: "click", currentTarget: this.buttonElement },
      { name: "log", identifier: "c", eventType: "click", currentTarget: this.element }
    )
  }

I modified it to add a :prevent modifer

async "test adding an action to the left with a :prevent modifier"() {
    this.actionValue = "c#log3:prevent c#log d#log2"
    await this.nextFrame
    await this.triggerEvent(this.buttonElement, "click")

    console.log(this.actionLog.map(({ name }) => name))
    this.assertActions(
      { name: "log3", identifier: "c", eventType: "click", currentTarget: this.buttonElement },
      { name: "log", identifier: "c", eventType: "click", currentTarget: this.buttonElement },
      { name: "log2", identifier: "d", eventType: "click", currentTarget: this.buttonElement },
      { name: "log", identifier: "c", eventType: "click", currentTarget: this.element }
    )
  }

The test fails events are received in this order
['log', 'log2', 'log3', 'log']
whereas it should be
['log3', 'log', 'log2', 'log']

Running the same test with #684 works. I'll what we can do to get this PR merged. While it is not a complete solution think it solves the vast majority of this issue

@adrienpoly adrienpoly mentioned this issue Aug 31, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@janko @adrienpoly