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.

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

Emit warnings for undefined controllers, actions and targets #413

Merged
merged 5 commits into from
Jun 30, 2021

Conversation

marcoroth
Copy link
Member

This PR allows Stimulus to emit warnings if a controller, action or target name is misspelled, incorrect or non-existent.

Previously Stimulus just ignored the wrong declared objects and just did nothing which makes it especially hard for newcomers to grok what's going on. This PR helps in that matter because it emits warnings upfront.

The following scenarios are covered and emit a warning if:

  • The declared controller in the data-controller attribute doesn't exist or isn't registered in the Application
  • The declared method in the data-action attribute doesn't exist on the given controller
  • The declared controller in the data-action attribute doesn't exist
  • The declared target in a data-target or data-[identifier]-target doesn't exist in the static targets array of the controller.
  • The declared controller in a data-target doesn't exist
  • The data-[identifier]-target attribute is empty
  • The format of the data-target attribute is wrong or if the value is empty

You can also silence the warnings by setting warnings to false:

  const application = Application.start()
  const context = require.context("./controllers", true, /\.js$/)
  application.load(definitionsFromContext(context))
+ application.warnings = false

Resolves #324

@existentialmutt
Copy link

Good call! Been using stimulus for years and debugging typoed attributes is still a big time suck.

@leastbad
Copy link
Contributor

Thank goodness. 👏

@dhh dhh merged commit 7768214 into hotwired:main Jun 30, 2021
@marcoroth marcoroth deleted the warnigns branch July 19, 2021 21:01
@dhh
Copy link
Member

dhh commented Sep 22, 2021

@marcoroth Trying to use this in anger right now, and I'm seeing that this is causing false warnings for undefined controllers when connecting actions on what appears elements that declare multiple controllers. Seems like when the first controller is detected, the actions on that element are all inspected, and if one of the actions refer to one of the other controllers mention on that element, then you get an error. Haven't fully traced it down yet.

@dhh
Copy link
Member

dhh commented Sep 22, 2021

Here's a test case:

<html>
<head>
  <script type="module">
    import { Application, Controller } from "https://ga.jspm.io/npm:@hotwired/stimulus@3.0.0-beta.2/dist/stimulus.js"

    const application = Application.start()

    application.register("first", class extends Controller {
      hello() {
        alert("hello!")
      }
    })

    application.register("second", class extends Controller {
      goodbye() {
        alert("goodbye!")
      }
    })
  </script>
</head>
<body>
  <div data-controller="first second" data-action="click->first#hello click->second#goodbye">
  </div>
</body>
</html>

Produces:
Screen Shot 2021-09-22 at 16 32 00

@porl
Copy link

porl commented Oct 28, 2021

@dhh I also get this same behaviour with "nested" controllers - if I remove the top controller I get no warnings (so I know it isn't a typo etc). But when there is a top "outer" controller then the actions on the inner controller trigger warnings. I am assuming they are being parsed when stimulus is initialising the first outer controller.

@dhh
Copy link
Member

dhh commented Oct 28, 2021

Warnings have been removed from the 3.0 release. They need more time in the oven.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

No error when target or controller is incorrect
5 participants