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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Namespaced Custom Events #28491

Open
daniel-nagy opened this issue Feb 1, 2019 · 8 comments
Open

[Bug]: Namespaced Custom Events #28491

daniel-nagy opened this issue Feb 1, 2019 · 8 comments
Labels
area: core Issues related to the framework runtime breaking changes core: event listeners design complexity: major freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Milestone

Comments

@daniel-nagy
Copy link

馃悶 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 daniel-nagy changed the title Namespaced Custom Events [Bug]: Namespaced Custom Events Feb 1, 2019
@daniel-nagy
Copy link
Author

daniel-nagy commented Feb 1, 2019

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
Copy link
Author

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>

@pkozlowski-opensource pkozlowski-opensource added the area: core Issues related to the framework runtime label Feb 4, 2019
@ngbot ngbot bot added this to the needsTriage milestone Feb 4, 2019
@robwormald
Copy link
Contributor

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
Copy link
Contributor

trotyl commented Feb 6, 2019

@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
Copy link
Contributor

mhevery commented Feb 12, 2019

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

@pkozlowski-opensource
Copy link
Member

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.

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed severity3: broken labels Oct 1, 2020
@tonjohn
Copy link

tonjohn commented Dec 21, 2023

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
Copy link

tonjohn commented Dec 21, 2023

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues related to the framework runtime breaking changes core: event listeners design complexity: major freq1: low P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: confirmed type: bug/fix
Projects
None yet
Development

No branches or pull requests

8 participants