Skip to content

fix(platform-server): Do not delete global Event#53659

Closed
atscott wants to merge 2 commits intoangular:mainfrom
atscott:dominoEvent
Closed

fix(platform-server): Do not delete global Event#53659
atscott wants to merge 2 commits intoangular:mainfrom
atscott:dominoEvent

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Dec 20, 2023

This commit removes a hack that deletes Event from the global context when using domino. Instead, it sets the global event to domino's implementation of Event.

The "Trick to avoid Event patching" links to a state in the repository that is 6(!) years old:

private patchEvent() {
if (!Event || !Event.prototype) {
return;
}
if ((Event.prototype as any)[stopMethodSymbol]) {
// already patched by zone.js
return;
}
const delegate = (Event.prototype as any)[stopMethodSymbol] =
Event.prototype.stopImmediatePropagation;
Event.prototype.stopImmediatePropagation = function() {
if (this) {
this[stopSymbol] = true;
}
// should call native delegate in case
// in some enviroment part of the application
// will not use the patched Event
delegate && delegate.apply(this, arguments);
};
}
. A lot has changed since then. I don't know what the implications might be with this change but @tbondwilkinson and I are encountering issues with events when working with the navigation API that are caused by this bit of hackery.

This commit removes a hack that deletes `Event` from the global context
when using domino. Instead, it sets the global event to domino's
implementation of `Event`.
@atscott atscott marked this pull request as ready for review January 4, 2024 18:07
@atscott atscott requested a review from AndrewKushnir January 4, 2024 18:07
@pullapprove pullapprove bot requested a review from devversion January 4, 2024 18:08
@atscott atscott added area: testing Issues related to Angular testing features, such as TestBed target: patch This PR is targeted for the next patch release labels Jan 4, 2024
@ngbot ngbot bot modified the milestone: Backlog Jan 4, 2024
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jan 4, 2024
@atscott
Copy link
Copy Markdown
Contributor Author

atscott commented Jan 4, 2024

This PR was merged into the repository by commit f4bd5a3.

@atscott atscott closed this in f4bd5a3 Jan 4, 2024
atscott added a commit that referenced this pull request Jan 4, 2024
This commit removes a hack that deletes `Event` from the global context
when using domino. Instead, it sets the global event to domino's
implementation of `Event`.

PR Close #53659
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This commit removes a hack that deletes `Event` from the global context
when using domino. Instead, it sets the global event to domino's
implementation of `Event`.

PR Close angular#53659
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
This commit removes a hack that deletes `Event` from the global context
when using domino. Instead, it sets the global event to domino's
implementation of `Event`.

PR Close angular#53659
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
This commit removes a hack that deletes `Event` from the global context
when using domino. Instead, it sets the global event to domino's
implementation of `Event`.

PR Close angular#53659
@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: testing Issues related to Angular testing features, such as TestBed target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants