feat(core): add event listener options to renderer#59092
feat(core): add event listener options to renderer#59092crisbeto wants to merge 1 commit intoangular:mainfrom
Conversation
Updates the `Renderer2.listen` signature to accept event options, as well as all adjacent types to it.
db372e9 to
df09f85
Compare
| }): WritableSignal<D>; | ||
|
|
||
| // @public | ||
| export interface ListenerOptions { |
There was a problem hiding this comment.
Initially I wanted to call this EventListenerOptions, but that's the same name as the native type.
| "name": "InputFlags" | ||
| }, | ||
| { | ||
| "name": "KeyEventsPlugin" |
There was a problem hiding this comment.
Why do we start to get those symbols all of the sudden?
There was a problem hiding this comment.
Not sure, but it's weird given that I only added a type and type-only imports. I approved the diffs, because I think that those symbols have always been there. AFAIK we don't have a way to tree shake them even if we wanted to.
There was a problem hiding this comment.
I spent some time going through all the changes, and I'm convinced that this is an infra quirk, rather than the symbols changing. As soon as I add any parameter to DomEventsPlugin.removeEventListener, the bundle golden needs to be updated. Here's a minimal repro: crisbeto@bf50e54. All I did was add the options parameter, pass the parameter into the removeEventListener callback and run yarn bazel run //packages/core/test/bundling/standalone_bootstrap:symbol_test.accept.
There was a problem hiding this comment.
What's suprising to me is that they weren't here before. Them being being listed is actually normal.
There was a problem hiding this comment.
Agreed. The DomEventsPlugin is provided eagerly here https://github.com/angular/angular/blob/main/packages/platform-browser/src/browser.ts#L238. I don't think there's a way to tree shake it away from there. There's also the fact that if it was tree shaken, Angular wouldn't be able to bind events in the browser.
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
|
This PR was merged into the repository by commit d010e11. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Updates the `Renderer2.listen` signature to accept event options, as well as all adjacent types to it. PR Close angular#59092
Updates the
Renderer2.listensignature to accept event options, as well as all adjacent types to it.