fix(ng-add): allow using noop animations #13429
Conversation
devversion
added
pr: merge safe
target: major
googlebot
added
the
cla: yes
|
|
||
| describe('animations disabled', () => { | ||
|
|
||
| it('should not add @angular/animations to package.json', () => { |
There was a problem hiding this comment.
Isn't this always required, though, since @angular/material imports stuff from @angular/animations?
There was a problem hiding this comment.
I think it depends from where people import. Most imports from @angular/animations are just types that won't be part of the JS.
My reasoning for this option was just that there might be people who don't want the schematics to touch their package.json/app.module.ts. For the general public, I think it would be better if we just add the NoopAnimationsModule or BrowserAnimationsModule and always ensure @angular/animations is set up?
There was a problem hiding this comment.
It's probably worth checking, but I'm reasonable sure that the app won't build if @angular/animations isn't installed. I don't want there to be any paths where the user ends up with an app that doesn't build.
There was a problem hiding this comment.
Yeah, it would be wrong to open any "path" where the user ends up with an application that doesn't build.
I've updated this PR to either add the NoopAnimationsModule or BrowserAnimationsModule. This also includes a safety check that ensures that we don't "magically" add the BrowserAnimationsModule or NoopAnimationsModule if the project already explicitly uses an animation module. Please have another look.
devversion
force-pushed
the
feat/ng-add-allow-disabling-animations
branch
3 times, most recently
from
0398dce to
36f448d
Compare
devversion
changed the title
* Currently we always install `@angular/animations` and add the `BrowserAnimationsModule` to the project module. There should be a prompt/option that allows developers to use the `NoopAnimationsModule`. * Also no longer adds the `BrowserAnimationsModule` if the project already uses the `NoopAnimationsModule` (avoiding magic). Same applies for the `NoopAnimationsModule`. We won't add the noop animations module if the browser animations module is configured. Handle existing animation modules
devversion
force-pushed
the
feat/ng-add-allow-disabling-animations
branch
from
36f448d to
2ac6eaf
Compare
devversion
changed the title
* Currently we always install `@angular/animations` and add the `BrowserAnimationsModule` to the project module. There should be a prompt/option that allows developers to use the `NoopAnimationsModule`. * Also no longer adds the `BrowserAnimationsModule` if the project already uses the `NoopAnimationsModule` (avoiding magic). Same applies for the `NoopAnimationsModule`. We won't add the noop animations module if the browser animations module is configured. Handle existing animation modules
|
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. |
Currently we always install
@angular/animationsand add theBrowserAnimationsModuleto the project module. There should be a prompt/option that allows developers to use theNoopAnimationsModule.No longer adds the
BrowserAnimationsModuleif the project already uses theNoopAnimationsModule(avoiding magic). Same applies for theNoopAnimationsModule. We won't add the noop animations module if the browser animations module is configured.