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

fix(platform-browser): DomEventsPlugin should always be the last plugin to be called for supports(). #50394

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeanMeche
Copy link
Member

This fixes the issues when BrowserModule is not the first module imported.

I choose not to introduce a specific token for the DomEventsPlugin as this would likely be breaking.

Fixes #37149 #37850

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • No

@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch 2 times, most recently from f223c82 to 60a97de Compare May 21, 2023 12:46
@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch 2 times, most recently from 57065c6 to fdab3b4 Compare May 21, 2023 13:06
@JeanMeche JeanMeche marked this pull request as ready for review May 21, 2023 13:06
@pullapprove pullapprove bot requested a review from alxhub May 21, 2023 13:06
@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch from fdab3b4 to 9290de1 Compare May 21, 2023 13:25
@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels May 22, 2023
@ngbot ngbot bot modified the milestone: Backlog May 22, 2023
@jessicajaniuk jessicajaniuk added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: merge The PR is ready for merge by the caretaker labels May 22, 2023
@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch 5 times, most recently from 623f32e to 8edee8b Compare June 9, 2023 22:40
@JeanMeche JeanMeche requested a review from alxhub June 9, 2023 22:58
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the g3 failures, it looks like g3 expects to import EventManagerPlugin from the event_manager.ts file. We could update g3 when we land this change, but I don't think there's a reason to separate EventManagerPlugin into a separate file anymore - how would you feel about keeping it in event_manager.ts?

@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch from 8edee8b to beaca5d Compare June 13, 2023 05:14
@JeanMeche
Copy link
Member Author

It makes sense, we're down to 3 changed files.

@JeanMeche JeanMeche requested a review from alxhub June 27, 2023 21:22
@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch from beaca5d to ffdc83f Compare August 29, 2023 17:04
…ugin to be called for `supports()`.

This fixes the issues when `BrowserModule` is not the first module imported.

Fixes angular#37149 angular#37850
@JeanMeche JeanMeche force-pushed the fix/event-manager-plugins-order branch from ffdc83f to 2495ce8 Compare August 29, 2023 17:21
@dylhunn dylhunn self-requested a review September 19, 2023 16:46
@JeanMeche JeanMeche closed this Feb 29, 2024
@JeanMeche JeanMeche deleted the fix/event-manager-plugins-order branch February 29, 2024 13:40
@JeanMeche JeanMeche restored the fix/event-manager-plugins-order branch March 1, 2024 00:08
@JeanMeche JeanMeche reopened this Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HammerModule does not work when imported before BrowserModule
4 participants