fix(core): run ApplicationRef.prototype.bootstrap in NgZone#60720
fix(core): run ApplicationRef.prototype.bootstrap in NgZone#60720dgp1130 wants to merge 1 commit intoangular:mainfrom
ApplicationRef.prototype.bootstrap in NgZone#60720Conversation
atscott
left a comment
There was a problem hiding this comment.
I don't know if I think this is worth mentioning as a breaking change especially if there were no failures in TGP
|
Bootstrap isn't typically run in unit tests so I'm not entirely sure how much confidence TGP gives us (e2e tests should exercise this in some capacity). Clearly it wasn't that breaking in production. Happy to defer to you or other FW folks if this is worth mentioning as a breaking change or not. |
157c70d to
420dc15
Compare
There's only 1 place in g3 that appears that it could be running bootstrap outside the app's zone. I can't possibly imagine this is intentional or that it would break things to run it (correctly) inside the zone. If it was actually possible to keep things outside the zone, then maybe I could see it. But any time the application runs change detection automatically, it's going to run it inside the zone for that component, even if it was bootstrapped outside the zone. I would personally omit the breaking change note since it would just create unnecessary noise. We can still land it in major instead of patch though if you want to be extra safe. |
`bootstrapApplication` always creates the root component in the `NgZone`, however `ApplicationRef.prototype.bootstrap` historically did not, meaning that if users did not go out of their way to call `ngZone.run(() => appRef.bootstrap(SomeComp))`, components would not run change detection correctly. This commit updates `ApplicationRef.prototype.bootstrap` to _always_ run within `NgZone`, removing this hazard and ensuring components always run CD as expected.
Removed the breaking change note. I feel like you understand the implications of this better than I do, so I'm happy to defer to you whether we should merge to |
|
This PR was merged into the repository by commit 0ae1889. 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. |
bootstrapApplicationalways creates the root component in theNgZone, howeverApplicationRef.prototype.bootstraphistorically did not, meaning that if users did not go out of their way to callngZone.run(() => appRef.bootstrap(SomeComp)), components would not run change detection correctly.This commit updates
ApplicationRef.prototype.bootstrapto always run withinNgZone, removing this hazard and ensuring components always run CD as expected.TGP: http://test/OCL:743313795:BASE:743313664:1743645912499:dad01837