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

Add Controller#shouldLoad #447

Closed
dhh opened this issue Sep 21, 2021 · 3 comments
Closed

Add Controller#shouldLoad #447

dhh opened this issue Sep 21, 2021 · 3 comments

Comments

@dhh
Copy link
Member

dhh commented Sep 21, 2021

We use Controller#shouldLoad to prevent certain controllers from being loaded given environmental conditions, like native vs web, working around browser issues, etc. Would be good to get this into Stimulus proper. Here's the dynamic hack version of it I was just using in Basecamp:

application.register = function(identifier, controllerConstructor) {
   if (controllerConstructor.shouldLoad) {
     this.load({ identifier: identifier, controllerConstructor: controllerConstructor })
   }
 }.bind(application)
@seanpdoyle
Copy link
Contributor

Since controllers already rely on the presence of absence of their identifier in a data-controller attribute, could the goals of a static shouldLoad() { ... } method be achieved in other ways?

For example, on the server side with something like token_list:

<div data-controller="<%= token_list "always", "when-mobile" => request.variant.mobile?, "when-admin" => Current.user.admin? %>"></div>

On the client side, conditions could be checked in the initialize() hook to determine whether or not to remove the identifier:

import { controllerIsDisabled } from "../utils"

export default class extends Controller {
  initialize() {
    if (controllerIsDisabled()) {
      const { dataset } = this.element
      dataset.controller = dataset.controller.replaceAll(this.identifier, "")
    }
  }
}

On a related note, I'm curious if the presence of an unloaded element could affect Stimulus' controller resolution or other DOM focussed operations (for example, MutationObserver instances).

Is there a consideration I'm misunderstanding here?

@dhh
Copy link
Member Author

dhh commented Sep 21, 2021

Monkeying with the dom would bust caching, so that's why we do the alternation in the JS.

@dhh
Copy link
Member Author

dhh commented Sep 21, 2021

We've been running with this setup since before Turbo was released on HEY. We just had it as part of a webpack helper before. So it never made it upstream.

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

No branches or pull requests

2 participants