angular / angular

Deliver web apps with confidence πŸš€
https://angular.dev
MIT License
96.34k stars 25.56k forks source link

[Bug]: Namespaced Custom Events #28491

Open daniel-nagy opened 5 years ago

daniel-nagy commented 5 years ago

🐞 bug report

Affected Package

platform-browser

Is this a regression?

No. ### Description

Custom events that are namespaced using the syntax namespace:event will fail. For example, binding to the MDCTabBar:activated event will fail (source).

<mwc-tab-bar (MDCTabBar:activated)="onTabActivated($event)"></mwc-tab-bar>

The problem is Angular will think you are listening to an activated event on a global MDCTabBar event target (source).

Possible Solution

If Angular doesn't know about the event target then assume it is part of the event name, or add syntax to tell Angular to listen to the event name verbatim. I think I saw the ^ syntax in another issue.

e.g.

<!-- hey Angular, don't mess with the event name -->
<mwc-tab-bar (^MDCTabBar:activated)="onTabActivated($event)"></mwc-tab-bar>

πŸ”¬ Minimal Reproduction

Not needed.

πŸ”₯ Exception or Error


Unsupported event target null for event activated

🌍 Your Environment

Angular Version:


platform-browser: 7.0.3

Anything else relevant?

daniel-nagy commented 5 years ago

Here is where it does the parsing: packages/compiler/src/template_parserbinding_parser.ts#L335

Edit: Something like this would work:

let target: string | null = null;
let eventName: string;

if (name[0] === '^') {
  eventName = name.slice(1)
else {
  ([target, eventName]) = splitAtColon(name, [null !, name]);
}
daniel-nagy commented 5 years ago

If anyone lands here in the future, I came up with this ugly work around.

import { DOCUMENT, EventManager, EVENT_MANAGER_PLUGINS } from '@angular/platform-browser';

class RewriteEventManagerPlugin {
  manager: EventManager;

  constructor(private _doc: Document) {

  }

  supports(eventName: string): boolean {
    return eventName.indexOf('@') !== -1;
  };

  addEventListener(element: HTMLElement, eventName: string, handler: Function): Function {
    return this.manager
      .addEventListener(element, eventName.replace('@', ':'), handler);
  }
}

@NgModule({
  ...
  providers: [
    {
      multi: true,
      provide: EVENT_MANAGER_PLUGINS,
      useFactory(document: Document) {
        return new RewriteEventManagerPlugin(document);
      },
      deps: [DOCUMENT]
    }
  ]
})
class MyModule {}
<!-- the @ symbol will be replace with a ":" when the event listener is added to the element -->
<mwc-tab-bar (MDCTabBar@activated)="onTabActivated($event)"></mwc-tab-bar>
robwormald commented 5 years ago

As MDCTabBar:activated is a completely valid (if surprising!) DOM Event name, I don't think we should have a special syntax (like (^MDCTabBar:activated) - we should check against a known set (document,window,etc) and otherwise, not mess with it and pass it through...

trotyl commented 5 years ago

@robwormald In that sense, document:activated as well as window:activated are also completely valid DOM Event names, why shouldn't them being allowed?

mhevery commented 5 years ago

Would love to have a PR for this (as well as a test)

pkozlowski-opensource commented 4 years ago

Re-confirming what was already said - an event with : in its name (ex. MDCTabBar:activated) is a valid event name and some implementations use it!

The bad part here is that Angular uses : to denote a target to add an event to. Changing meaning of : at this point would be a massive breaking change. Not sure how to solve it but this is clearly a bug from a user's point of view.

tonjohn commented 11 months ago

Any updates? We make extensive use of Lit web components on https://shop.battle.net/ which emit events that contain : (example, blz-video:state-change). It's frustrating that we have to manually add listeners instead of leveraging Angular's binding syntax.

tonjohn commented 11 months ago

I found a workaround! Looking at the code, I noticed _splitAt() only checks the first instance of character (which is : in this case). So we just have to prefix the event with : to trick the parser.

Using the example from the OP, the workaround looks like:

<mwc-tab-bar (:MDCTabBar:activated)="onTabActivated($event)"></mwc-tab-bar>