-
Notifications
You must be signed in to change notification settings - Fork 26.5k
refactor(animations): remove dependency on @angular/common
#63248
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
00ce1d8
to
3b13096
Compare
3b13096
to
a9e7fd9
Compare
Deployed adev-preview for a8d701d to: https://ng-dev-previews-fw--pr-angular-angular-63248-adev-prev-ttun7f7r.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
`DOCUMENT` was move to `@angular/core`, so we don't need to depend on common anymore.
a9e7fd9
to
a8d701d
Compare
I build locally the integration project locally, could find any major difference to explain the size increase. It must be pre-exisiting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
reviewed-for: fw-general, size-tracking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Just the one question about making sure we are good with the size diff
@@ -1,5 +1,5 @@ | |||
{ | |||
"dist/main.js": 86540, | |||
"dist/main.js": 97227, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems extremely counter intuitive for a change where we are removing a dependency? Is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We spent some time looking at it, but it doesn't appear to be coming from this PR.
DOCUMENT
was move to@angular/core
, so we don't need to depend on common anymore.