hotwired / stimulus

A modest JavaScript framework for the HTML you already have
https://stimulus.hotwired.dev/
MIT License
12.7k stars 424 forks source link

Outlet can't have custom name #641

Open amkisko opened 1 year ago

amkisko commented 1 year ago

It's not clear from reference documentation that outlet name has to be same as referenced controller identifier.

Also for some reason logic behind matching element does not work properly. I tried using outlets, it recognises the controller by identifier (same way if I use Stimulus.router.module_by_identifier.get), but can't match the element (I did not dig deeper).

tmbv93 commented 1 year ago

Furthermore, since we require the outlet to have a controller with the same name as the outlet, isn't having a CSS selector a bit redundant? It would be simpler to just define the outlets like this:

Parent controller HTML: <div data-controller="search">

Parent controller JS: static outlets = ['result']

Outlet HTML: <div data-controller="result">

If we used this syntax, it would mean that all elements with data-controller="result" would automatically become outlets, but that seems like a small trade-off for simpler syntax.

If outlets didn't need to have a controller with a matching identifier, it would be much easier to see the use case for CSS selectors. You could have several different controllers become outlets

Parent controller HTML: <div data-controller="search" data-search-result-outlet=".result">

Parent controller JS: static outlets = ['result']

Outlet: <div data-controller="result somethingelse" class="result">

A bit tangential, but: I think it would be neat if both result_controller.js and somethingelse_controller.js in this example became outlets. It would allow for a listener pattern, where several types of elements can define custom behavior in response to actions done in the parent controller.

I'm a bit new to using Outlets, so sorry if I'm just misunderstanding the Outlets API completely!

marcoroth commented 1 year ago

We probably could also offer a way to rename the outlet name, so that it doesn't need to be the controller identifier of the outlet controller. Previously we had the option as an array:

static outlets = ["result"]

But we could probably also support a syntax with an object. The example above would be a shorthand for:

static outlets = {
  "result": "result"
}

But with this syntax you could rename the outlet name using [outlet controller identifier]: [outlet name in the host controller]:

static outlets = {
  "result": "anotherNameForResult"
}

So a call like:

this.resultOutlets

would become:

this.anotherNameForResultOutlets

The same would apply for the data-[identifier]-[outlet name]-outlet you need to provide.

What do you think?


@tmbv93 I actually have a pull request in progress to solve the first part you are describing, so that it falls back to [data-controller~={outlet}] if you didn't provide a more specific selector for the outlet.

amkisko commented 1 year ago

I am not even sure if outlets even correct pattern, it is possible one, but all the problems that start to appear seem to bring more trouble than value. Is there any way to utilise some pattern like service in Angular? Something intermediary that lives outside both controllers and initialises somewhere on entrypoint level.

gato-omega commented 1 year ago

After having some issues understanding the outlet API and which naming conventions to use, I found these issues and what I have experienced seems to confirm the "outlet [can't have | is not a] custom name" and "outlet name must be identical to controller identifier". Now I am currently sitting at this warning/error: The provided outlet element is missing the outlet controller "very--nested--identifier" for "very--nested--host"

But I can clearly see the data-controller matches fine, so... not really sure if this perhaps something I am still misunderstanding (probably) or if there's some kind of flaw regarding outlets. But in any case it seems to arise from the expected naming conventions.

drewlustro commented 1 year ago

@gato-omega – you're not alone. It's not working for me either. The parent controller can't seem to find the nested outlet.

taylorthurlow commented 1 year ago

I'm also trying to work through my own confusion with the outlets feature - finally understanding that outlets must be named (in the host controller) identically to the guest controller's identifier.

@gato-omega and @drewlustro - I had this same issue but I think I may have come to the conclusion that the problem I was having (the same error message as gato-omega's, I am using nested/namespaced controllers) is actually the result of my debugging console logs, and placing them in connect() of the host controller. Because connect() is called as soon as the controller is connected to, that means the guest controllers have not been instantiated/connected yet. Here's a short snippet to explain the symptom:

connect() {
  console.debug(this.fooBarBazOutlets);

  setTimeout(() => {
    console.debug(this.fooBarBazOutlets);
  }, 1000);
}

The first log triggers the warning that gato-omega posted one or more times, once per guest outlet controller (that is not yet connected). The second log prints the outlets with no issues, because 1 second later, the rest of the controllers have connected already.

After writing all this I've now found #618 which is the problem I'm describing. I'm not really sure at this point if nested/namespaced controllers are causing any problems.

benoittgt commented 1 year ago

You should also my pending PR. That helps to understand outlets API https://github.com/hotwired/stimulus/pull/663

amkisko commented 11 months ago

Still could not make this work, used the other non-recommended way that works fine for now and can be improved by using rxjs:

  connect(): void {
    Promise.resolve().then(() => {
      const controllerName = "online-users";
      const outlet = document.querySelector(
        `[data-controller='${controllerName}']`,
      ) as HTMLElement;
      if (!outlet) return;

      this.onlineUsersOutlet =
        this.application.getControllerForElementAndIdentifier(
          outlet,
          controllerName,
        ) as OnlineUsersController;
    });
  }
Rykus0 commented 10 months ago

Agree that this is 100% not clear in the docs. I lost at least a half a day to this.

This can also be a problem if you have two outlets using the same controller, but that need to be handled separately. I suppose you could loop through them and then check an ID or something, but I'd rather explicitly define each outlet.

tacman commented 6 months ago

I was trying to use an outlet that's in a Symfony bundle, defined in a global twig variable. It's almost impossible to do without a custom name. The @@ is to escape the @, but this gets converted into survos--mobile-bundle-mobile, which if I then have to pass that to the controller as the outlet name, it has to be hard-coded in order to then do anything with the outlet.

So a custom name, or some sort of aliasing, would be most welcome.

twig:
    globals:
#        _app_sc: '@@survos/mobile-bundle/mobile'
        _app_sc: 'app'