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

Outlets array expects kebab-cased names instead of camel-cased ones (like targets do) #615

Open
wynksaiddestroy opened this issue Nov 30, 2022 · 13 comments

Comments

@wynksaiddestroy
Copy link

If you have a target name which consists of multiple words, you'll define your targets as follows:

static targets = [ "errorMessage" ]

An outlet name with multiple words, has to be defined the following way:

static outlets = [ "error-message" ]

If this is the intended behaviour, it would be nice to add a small notice to the documentation. Thanks!

@marcoroth
Copy link
Member

Hey @wynksaiddestroy, thanks for opening this issue!

This is a good point, and I think I agree with you that it's kinda unexpected if you look at how targets are defined.

I'm wondering why I did it that way. I guess because controller identifiers are also being written in kebab-case, and since outlets reference controller identifiers I probably went with that.

I guess you could expect both ways, depending on how you look at it.

Not sure how we should proceed here. I'd be fine with changing it. But that would probably also mean it's a breaking change.

@wynksaiddestroy
Copy link
Author

The Naming Conventions section of the Values reference page also says:

Write value names as camelCase in JavaScript and kebab-case in HTML.

So I guess, using camel-case for outlet names would keep everything more consistent than using kebab-case. But I would be satisfied with an updated documentation as well. Mabye someone else has an opinion on this issue?

@marcoroth
Copy link
Member

Good point! I'd be happy to change it, but would also love to hear feedback from others.

@ghost
Copy link

ghost commented Nov 30, 2022

Agree it should be changed for consistency. Might as well be now while the feature is new.

@xmas7
Copy link

xmas7 commented Nov 30, 2022

Totally agreed! Outlets better to use kebab-case!
Not sure if its a MUST but assuming that most folks like to use that one and its gonna be a Good practice or something.

@dhh
Copy link
Member

dhh commented Nov 30, 2022

Would be happy to ship a quick 3.2.2 that addresses this. If there was a way to detect kebab case, still have it work with a warning of depreciation, and then document and recommend camel going forward, that would be great.

@minimul
Copy link

minimul commented Dec 1, 2022

I like kabab-case and wish targets worked as outlets do it that regard – but yeah consistency overrules.

@NakajimaTakuya
Copy link
Contributor

NakajimaTakuya commented Dec 2, 2022

I generally agree with the alignment with the targets api.
However, I have one concern.
If outlets are written in CamelCase, there is no way to distinguish between -- and -.

before:

// in html
data-controller="hoge"
data-hoge-xxx-yyy-outlet="#zzz"
data-hoge-xxx--yyy-outlet="#ppp"

// in controller
static outlets = ['xxx-yyy', 'xxx--yyy']

after

// in html
data-controller="hoge"
data-hoge-xxx-yyy-outlet="#zzz"
data-hoge-xxx--yyy-outlet="#ppp"

// in controller
static outlets = ['xxxYyy', 'xxxYyy'] // I do not know which of these refers to which.

Originally, outlet getter had this problem.
However, it used to be discernible at the time of definition and it was predictable that the two would be mixed.

Wouldn't it be difficult to work backwards from the outlets static property to the controller name, given the existence of -- and - in the first place?

@drjayvee
Copy link
Contributor

drjayvee commented Dec 2, 2022

This may be unrelated, but the docs leave me wondering whether you need to use the outlet controller name verbatim, or whether you're allowed to alias it.

<div class="stuff" data-controller="awesome-stuff"></div>

<!-- according to the docs: -->
<div
  data-controller="search"
  data-search-awesome-stuff-outlet=".stuff"
>

<!-- is the following allowed as well? -->
<div
  data-controller="search"
  data-search-stuff-outlet=".stuff"
>

Would the 2nd then be allowed to use

class extends Controller {
  static outlets = ['stuff']
  declare readonly stuffOutlet: AwesomeStuffController
}

@rupasix
Copy link

rupasix commented Dec 15, 2022

Originally, outlet getter had this problem.
However, it used to be discernible at the time of definition and it was predictable that the two would be mixed.

Concerning is, if you use controllers in directory, your outlet controller names in camelcase are simplified, eg: search--form and search-form will be this.searchFormOutlet.

@minimul
Copy link

minimul commented Dec 23, 2022

@drjayvee

I tested this and it seems like the outlet name needs to be verbatim (to the controller you want to "outlet" to) and an alias doesn't work.

Is that what you found?

@drjayvee
Copy link
Contributor

@drjayvee

I tested this and it seems like the outlet name needs to be verbatim (to the controller you want "outlet" to) and an alias doesn't work.

Is that what you found?

@minimul yes, that's what I found as well.

@adiakritos
Copy link

I want to add to this that if you want to reference an outlet whose file name is something like:

foo/bar-controller

Then the html kebab case is needed when setting the outlet inside the controller like so:

static outlets = [ "foo--bar"].

Whether it's changed to match the pattern used by targets or not, I think the docs should speak to this since it's the current way newcomers will expect things to be given the pattern established by targets.

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

9 participants