feat(platform-browser): Expose EventManagerPlugin in the public api

[JeanMeche]

This commit exposes the EventManagerPlugin abstract class to the public API.
It provides a documentation with a usecase and a basic implementation describing the possibilities

Rationale for opening to the public API :

1. The EVENT_MANAGER_PLUGINS token is already public
2. The documentation provides a usefull usecase
3. Plugins can already be implemented by following the same API
4. This does not increase the API surface because of 3.
5. This topic gained some visibility in the last few days in the twitter community and seems to be not so widely known, so this helps also for the visibility ! (Also wanted to share the usescases implemented by @tinkoff/ng-event-plugins)

PR Type

What kind of change does this PR introduce?

• Feature

Does this PR introduce a breaking change?

• No

[JeanMeche]

Can I get the aio: preview ? 😊

[waterplea]

Super glad to see movement in this direction 👍

Just wanted to mention these two issues slightly impeding wide adoption of custom plugins:

#37850
#40553

[atscott]

I don't feel like the EventManager setting this property is a particularly good API. I understand that it's attempting to prevent a situation where the Zone is different but practically speaking, I don't think this ever happens. I would rather this be exposed as an interface that does not include manager.

[JeanMeche]

Great idea, that property bothered me also !

[waterplea]

Without that property, the example doesn't work, as well as the broader concept of unintrusive plugins that do their part and delegate further handling back to the manager which is heavily used by @tinkoff/ng-event-plugins.

[atscott]

They only don't work because of the way things are implemented currently. The could be refactored to work without the property being assigned by the EventManager itself.

That said, this was more of a discussion point because I don't necessarily think it's worth the churn to do this as a breaking change.

[JeanMeche]

The problem of accessing the manager can be solved by injecting it !

export class StopEventPlugin {
  constructor(private injector: Injector) {}

  supports(event: string): boolean {
    return event.endsWith('.stop');
  }

  addEventListener(element: HTMLElement, eventName: string, handler: (event: Event) => void): Function {
    const eventWrapper = (event: Event) => {
      event.stopPropagation();
      handler(event);
    };

    // striping the stop modifier from the event name;
    const nonModifiedEventName = eventName.replace(/\.stop$/, '');

    const manager = this.injector.get(EventManager);
    return manager.addEventListener(element, nonModifiedEventName, eventWrapper);
  }
}

Or we could run addEventListener in an injection context in the manager :

  addEventListener(element: HTMLElement, eventName: string, handler: Function): Function {
    const plugin = this._findPluginFor(eventName);
    return runInInjectionContext(this.injector, () => plugin.addEventListener(element, eventName, handler as (event: Event) => void));
  }

What do you think ?
Should we revert the breaking change or suggest to inject the manager if it's needed ?

[waterplea]

Personally, I don't think switching from property assignment by EventManager to the entire Injector to avoid cyclic dependency is much better. Sounds like for plugins to have direct access to the manager is reasonable.

[atscott]

Sounds like for plugins to have direct access to the manager is reasonable.

Having access to the manager isn't really the issue, but rather the awkwardness of having a public API where the some other service assigns a value to a user's publicly writable property after the service is created.

[waterplea]

Maybe use the same approach control value accessors have and add registerManager(manager: EventManager): void?

[atscott]

@JeanMeche Can you separate this in to two PRs? One that simply adds the @publicApi annotation to EventManagerPlugin and another that adds the documentation? For the public API change, you can simply cite the existing token: https://angular.io/api/platform-browser/EVENT_MANAGER_PLUGINS. The exposed type of the token should definitely be in the public API.

[JeanMeche]

@atscott I've reduced the scope of the PR to adding EventManagerPlugin to the public API.

Do you have any clue why getFactoryOf has been added to the goldens ?

[dylhunn]

reviewed-for: public-api

[github-actions]

Deployed aio for 2c61afe to: https://ng-dev-previews-fw--pr-angular-angular-49969-8pvqqo9w.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

[dylhunn]

@josephperrott I have a tooling question for you. I was trying to start a TGP, but g3sync fails with a patch conflict (albeit a very simple one). Is it possible to update the patch for my g3sync presubmit, without actually committing the patch update?

[josephperrott]

@dylhunn replying offline as this is a g3 specific item.

[dylhunn]

I will run a TGP on this CL tonight.

[dylhunn]

TGP is running: https://test.corp.google.com/OCL:560756113:BASE:560894821:1693279799412:6c3fde97
https://test.corp.google.com/OCL:560904996:BASE:560904861:1693283754070:13e6c6c9

[dylhunn]

TGP passed.

[jessicajaniuk]

This PR was merged into the repository by commit c5daa6c.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}