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

Camelcase Custom Events #918

Open
AngelMunoz opened this issue Dec 29, 2018 · 11 comments
Open

Camelcase Custom Events #918

AngelMunoz opened this issue Dec 29, 2018 · 11 comments

Comments

@AngelMunoz
Copy link

I'm submitting a feature request

  • Library Version:
    1.3.0

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    v10.10.0

  • NPM Version:
    6.4.1

  • Aurelia CLI OR JSPM OR Webpack AND Version
    Local aurelia-cli v1.0.0-beta.8 | webpack 4.27.0

  • Browser:
    all

  • Language:
    all

Current behavior:
Camelcase Custom Events are not handled by the binding engine

https://gist.run/?id=cc241040c915ecc7f8a80aaa97f9591f

Expected/desired behavior:
Camelcase Custom Events should be handled by the binding engine

  • What is the motivation / use case for changing the behavior?

Hello there, well I do know that Aurelia needs to declare camel-case attributes as dash case when writing html, but lately I came across this case where I'm using Ionic v4 (beta) which is based on stencil (ionic's web component compiler) so thanks to the fact that these are web components means I don't really need to do much in Aurelia to make them work, they just work and that's fine.

The main problem is that Ionic has adopted the idea of Emitting CustomEvents with camel-case based names in Aurelia we would usually have something like ion-change instead of ionChange. Ionic did this, so anyone else can also do that. That could make third party web components to not work as smoothly with Aurelia as any other Aurelia plugin

@EisenbergEffect
Copy link
Contributor

Thanks for bringing this up @AngelMunoz

@bigopon @fkleuver I think we should decided how we want to handle this with vNext first and then backport that API to vCurrent so that we have forward/backward consistency and make sure to address this the right way. Thoughts?

@fkleuver
Copy link
Member

fkleuver commented Dec 31, 2018

I took at look at stencil (the compiler behind ionic), they don't use native API's directly but use JSDom parser and have a VDom sitting in between that does the various translations. This gives them the ability to have a templating syntax that does not work in a normal browser environment.

We simply have the kebab-case convention to allow users to stick to conventions of what works in both native HTML and native JavaScript. It means that Aurelia code will also work outside of Aurelia. Frankly I wish more frameworks did that, but ionic chose to go the opposite way, alas.

You could use a binding behavior:

export class CamelCaseBindingBehavior {
  bind(binding) {
    binding.targetEvent = binding.targetEvent.replace(/\-(\w|$)/g, (_, x) => x.toUpperCase())
  }
}

And

<foo my-click.trigger="handleClick($event) & camelCase">

See the updated gist, it works: https://gist.run/?id=9c56883aa656ce6f2904f0c3b5293c2f

I suppose that's a fair kind of resource to have in the core framework.

We could also put a dispatchEvent api on the DOM object that automatically kebab-cases camelCased event names. Or even try to lowercase them as well, so you can use camelCased event names in the template too (browser already lowercases them). Not too sure about this though.

@AngelMunoz
Copy link
Author

@fkleuver Yeah I totally get the idea of the kebab-case my concern was about other users/authors looking at projects like ionic/stencil and grabbing the same conventions even though they don't adhere to the standard naming cases and since those conventions work for them in their use cases, there could be people thinking that Aurelia got it wrong even though it works as expected,

Note: Thanks for the snippet of the binding Behavior, never crossed my mind checked the gist and works just great! I will actually copy that one into the project I'm working with

@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Dec 31, 2018

Nice use of a binding behavior @fkleuver 😄

I think we wouldn't want to auto-camel case kebab-cased events without the developer having a way to opt-out and to also configure exceptions. There could be component vendors who use kebab-casing directly and we wouldn't want to break things the other way around.

Are there some new APIs that we should add to the EventManager?

@fkleuver
Copy link
Member

Are there some new APIs that we should add to the EventManager?

Well I thought about that but it seems like a somewhat impractical place to do it unless you want to do an app-wide override since it's a singleton.

We could introduce a decorator for event handlers. @bindable allows you to specify whatever attribute name you like, I don't see why a listener binding shouldn't have the same kind of configurability on that level.

e.g.

@eventHandler({ attribute: 'my-click', event: 'myClick' })
onClick($event) { ... }

@fkleuver
Copy link
Member

there could be people thinking that Aurelia got it wrong

I'll receive their issues/complaints with open arms as an opportunity to tell them how it really is ;)

But yeah it's not an ideal situation, that's for sure.

@EisenbergEffect
Copy link
Contributor

Something about that doesn't sit right. I get your argument about the parallel to binding but this still somehow seems like the wrong place for this.

It might be that we just build in a binding behavior and document it That's a legitimate solution. Then we can wait to see whether we come up with something better down the road.

@fkleuver
Copy link
Member

The thing is that events listener bindings are basically opposite property bindings.
In a property binding the attribute name is the target property and the attribute value binds to the source property.
For an event, the attribute name binds to the source property (the event name) and the attribute value to the target property (the event handler).
On one hand it made perfect sense to do this from the perspective of sticking to standards, but when you really think about it an event handler doesn't really need to be an expression. It's always a call expression that takes the event. Correct me if I'm wrong.
If the attribute value (the expression) mapped to the event name then that would give you all the freedom in the world in that respect. Binding behaviors and such would still work on the binding. You could do various things with the event, etc.

Maybe, maybe worth considering to move it around in vNext? It's a breaking change of a pretty large magnitude so a toggle would certainly be needed.

@StrahilKazlachev
Copy link
Contributor

For vCurrent can't we just show how to monkey patch or override the SyntaxInterpreter.prototype.trigger & Co.? Then the tryKebabToCamel could be applied here.
If this approach is an option, it can be done as a plugin.

@fkleuver
Copy link
Member

For folks who are okay with it being a global override I suppose that's okay. A fancy plugin could even add a little configuration api to specify which events to include or exclude from this transform (with a regex perhaps).
Well, seems like it's easy enough to fix one way or the other

@DarkAiR
Copy link

DarkAiR commented Dec 16, 2022

Behaviors looks like ValueConverters and can be attached to aurelia framework the same.

config.globalResources([PLATFORM.moduleName('./behaviors/index')]);
where ./behaviors/index is the index of your behaviors module.

Then you can use behavior globally in your template without requred
<my-component my-event.delegate="onEvent($event) & camelCase">

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

No branches or pull requests

5 participants