fix(@schematics/angular): update application schematics for module-based apps to use 'provideZoneChangeDetection'#32208
Conversation
…sed apps to use 'provideZoneChangeDetection'
Replaces deprecated 'ngZoneEventCoalescing' with 'provideZoneChangeDetection({ eventCoalescing: true })' in 'main.ts.template' in schematics for initial commit
| platformBrowser().bootstrapModule(AppModule, { | ||
| <% if(!zoneless) { %>ngZoneEventCoalescing: true,<% } %><% if(!!viewEncapsulation) { %> | ||
| defaultEncapsulation: ViewEncapsulation.<%= viewEncapsulation %><% } %> | ||
| <% if(!zoneless) { %> applicationProviders: [provideZoneChangeDetection({ eventCoalescing: true })], <% } %> |
There was a problem hiding this comment.
This provider should instead go in the module alongside provideBrowserGlobalErrorListeners: packages/schematics/angular/application/files/module-files/src/app/app__typeSeparator__module.ts.template
There was a problem hiding this comment.
Hi @atscott,
thank you for the review and for your comment!
My intention was to make this part of app the fresh module-based app look similar to the result of the bootstrap options migration
The following is an effect of the migration (tested ng20 -> ng21 and ng19 -> ng20 -> ng21):
Is there any particular reason why the migration does not remove deprecated ngZoneEventCoalescing and add provideZoneChangeDetection to the app module?
Of course, I'm happy to adjust code in the PR - just asking to get better context.
Thank you!
There was a problem hiding this comment.
add provideZoneChangeDetection to the app module?
yes. For the migration it is much harder to tell if there was already a change detection provider in the app module somewhere, including one of its imported modules. The migration would have had to instead create a separate module and add it to the imports to ensure it did not override any module providers.
There was a problem hiding this comment.
Thank you very much for sharing the context that helps me understand the decision!
I'll provide the adjustment soon.
There was a problem hiding this comment.
@atscott changes pushed 👍
I'd appreciate a review.
atscott
added
target: patch
|
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. |
3 participants
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the new behavior?
This PR provides changes in scaffolding module-based apps that removes usage of deprecated
ngZoneEventCoalescingand addsprovideZoneChangeDetection({ eventCoalescing: true })in root module, to be in line with changes from #30718.Does this PR introduce a breaking change?