fix(zone.js): move MutationObserver/FileReader to different module by JiaLiPassion · Pull Request #31657 · angular/angular

JiaLiPassion · 2019-07-19T14:15:46Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the new behavior?

Separate MutationObserver and FileReader to separated module, so they can be disabled separately.

Does this PR introduce a breaking change?

Yes
No

Close #36460

mhevery · 2019-07-22T22:51:28Z

http://test/OCL:259423218:BASE:259423249:1563835835152:2122d070

mhevery

This looks to me as a breaking change, since now the application may stop working. @IgorMinar

JiaLiPassion · 2019-07-22T23:42:36Z

@mhevery, yeah, you are right, if some developer disable EventTarget monkey patch, and before this PR, they will also disable MutationObserver and FileReader and now only EventTarget is disabled.

mhevery · 2020-07-10T18:45:06Z

presubmit
deflake

gkalpak · 2020-07-11T06:11:11Z

Since we are doing this, would it make sense to also separate IntersectionObserver? Or would that be too much granularity?

@gkalpak , sure, I have separate the IntersectionObserver into another module, thanks.

Separate `EventTarget`, `FileReader`, `MutationObserver` and `IntersectionObserver` patches into different module. So the user can disable those modules separately.

mhevery · 2020-07-16T16:49:34Z

@JiaLiPassion Could you add reasoning for this change.
@AndrewKushnir Please run global presubmit

AndrewKushnir · 2020-07-17T01:35:34Z

Presubmit + Global Presubmit.

JiaLiPassion · 2020-07-17T12:00:32Z

@mhevery , I forget to add the information of the motivation of this PR, some use cases require to disable MutationObserver patch but still keep the EventTarget patch, such as in this issue #36460.

AndrewKushnir · 2020-07-17T20:07:56Z

Quick update after running presubmit in Google's codebase:

there are no failing targets due to this change (which is great!)
I haven't found that the EventTarget patches were disabled for any apps, so this change should be pretty safe for Google's codebase

However since this is still a breaking change (for users who have the EventTarget patches disabled), this change should wait for the next major version (v11).

@mhevery please let us know if we should proceed with this change for v11.

Thank you.

mhevery · 2020-07-17T20:21:05Z

@mhevery please let us know if we should proceed with this change for v11.

I have discussed this with @jelbourn and the conclusion is that we can land this in the next minor release (not patch, and no need to wait until major)

AndrewKushnir · 2020-07-17T20:40:47Z

@JiaLiPassion just a small nit: it looks like this change is more a feature not a bugfix? May be change the commit message to feat(zone.js) ...?

…ngular#31657) Separate `EventTarget`, `FileReader`, `MutationObserver` and `IntersectionObserver` patches into different module. So the user can disable those modules separately. PR Close angular#31657

angular-automatic-lock-bot · 2020-08-24T16:16:38Z

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.}

…ngular#31657) Separate `EventTarget`, `FileReader`, `MutationObserver` and `IntersectionObserver` patches into different module. So the user can disable those modules separately. PR Close angular#31657

JiaLiPassion requested a review from a team July 19, 2019 14:15

googlebot added the cla: yes label Jul 19, 2019

mhevery reviewed Jul 22, 2019

View reviewed changes

mhevery added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jul 22, 2019

mhevery added state: blocked area: zones Issues related to zone.js labels Jul 25, 2019

ngbot Bot added this to the needsTriage milestone Jul 25, 2019

AndrewKushnir removed the action: merge The PR is ready for merge by the caretaker label Jan 28, 2020

AndrewKushnir modified the milestones: needsTriage, v10-candidates Jan 28, 2020

JiaLiPassion mentioned this pull request Apr 7, 2020

Provide a way for zonejs to not patch MutationObserver #36460

Closed

JiaLiPassion force-pushed the module branch from 2769361 to a97954e Compare June 15, 2020 03:54

JiaLiPassion requested a review from mhevery June 15, 2020 03:55

JiaLiPassion added the breaking changes label Jun 15, 2020

JiaLiPassion force-pushed the module branch 2 times, most recently from 1d11e99 to 8c7aa9f Compare June 15, 2020 06:55

JiaLiPassion removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 26, 2020

mhevery approved these changes Jul 10, 2020

View reviewed changes

gkalpak reviewed Jul 11, 2020

View reviewed changes

JiaLiPassion force-pushed the module branch 5 times, most recently from 276abb1 to 4dc1405 Compare July 13, 2020 14:40

fix(zone.js): move MutationObserver/FileReader to different module

a6b3309

Separate `EventTarget`, `FileReader`, `MutationObserver` and `IntersectionObserver` patches into different module. So the user can disable those modules separately.

JiaLiPassion force-pushed the module branch from 4dc1405 to a6b3309 Compare July 13, 2020 15:15

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Jul 17, 2020

AndrewKushnir modified the milestones: v10-candidates, v11-candidates Jul 17, 2020

AndrewKushnir added target: major This PR is targeted for the next major release and removed action: presubmit The PR is in need of a google3 presubmit target: patch This PR is targeted for the next patch release labels Jul 17, 2020

AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jul 20, 2020

mhevery removed the state: blocked label Jul 24, 2020

mhevery closed this in 253337d Jul 24, 2020

JiaLiPassion mentioned this pull request Aug 14, 2020

release: cut the zone.js-0.11.0 release #38473

Closed

12 tasks

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Aug 24, 2020

Conversation

JiaLiPassion commented Jul 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

mhevery commented Jul 22, 2019

Uh oh!

mhevery left a comment

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion commented Jul 22, 2019

Uh oh!

mhevery commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkalpak Jul 11, 2020

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion Jul 13, 2020

Choose a reason for hiding this comment

Uh oh!

mhevery commented Jul 16, 2020 • edited by AndrewKushnir Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewKushnir commented Jul 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JiaLiPassion commented Jul 17, 2020

Uh oh!

AndrewKushnir commented Jul 17, 2020

Uh oh!

mhevery commented Jul 17, 2020

Uh oh!

AndrewKushnir commented Jul 17, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Aug 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JiaLiPassion commented Jul 19, 2019 •

edited

Loading

mhevery commented Jul 10, 2020 •

edited

Loading

mhevery commented Jul 16, 2020 •

edited by AndrewKushnir

Loading

AndrewKushnir commented Jul 17, 2020 •

edited

Loading