fix(common): ensure diffing in ngStyle/ngClass correctly emits value changes #34307
Conversation
matsko
added
target: major
matsko
force-pushed
the
fix_ng_style_diffing
branch
from
c34afaa
to
3f565be
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
matsko
force-pushed
the
fix_ng_style_diffing
branch
from
3f565be
to
ba3fb1f
Compare
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
kara
added
action: review
|
@matsko you want this to go into v9, right? I changed the label to "master & patch" but let me know if that's not the case |
IgorMinar
force-pushed
the
fix_ng_style_diffing
branch
from
ba3fb1f
to
1e260c4
Compare
…changes Prior to this change, in Ivy mode ngStyle/ngClass would accidentally emit value changes for static (string-based) values even if the value itself had not changed. This patch ensures that the style/class diffing code is more strict and when it signals ngClass/ngStyle that there has been a value change. Fixes angular#34307
Since I was learning the codebase and had a hard time understanding what was going on I've done a bunch of changes in one commit that under normal circumstances should have been split into several commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to spend the time with trying to split this up. Overall I've done the following: - I processed review feedback from angular#34307 - I did a bunch of renaming to make the code easier to understand - I refactored some internal functions that were either inefficient or hard to read - I also updated lots of type signatures to correct them and to remove many casts in the code
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
…changes Prior to this change, in Ivy mode ngStyle/ngClass would accidentally emit value changes for static (string-based) values even if the value itself had not changed. This patch ensures that the style/class diffing code is more strict and when it signals ngClass/ngStyle that there has been a value change. Fixes angular#34336, angular#34444
Since I was learning the codebase and had a hard time understanding what was going on I've done a bunch of changes in one commit that under normal circumstances should have been split into several commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to spend the time with trying to split this up. Overall I've done the following: - I processed review feedback from angular#34307 - I did a bunch of renaming to make the code easier to understand - I refactored some internal functions that were either inefficient or hard to read - I also updated lots of type signatures to correct them and to remove many casts in the code
IgorMinar
force-pushed
the
fix_ng_style_diffing
branch
from
0ed7bf3
to
dafebe9
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
Since I was learning the codebase and had a hard time understanding what was going on I've done a bunch of changes in one commit that under normal circumstances should have been split into several commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to spend the time with trying to split this up. Overall I've done the following: - I processed review feedback from angular#34307 - I did a bunch of renaming to make the code easier to understand - I refactored some internal functions that were either inefficient or hard to read - I also updated lots of type signatures to correct them and to remove many casts in the code
IgorMinar
force-pushed
the
fix_ng_style_diffing
branch
from
dafebe9
to
189e111
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
| @@ -39,7 +39,7 @@ | |||
| "master": { | |||
| "uncompressed": { | |||
| "runtime-es2015": 2289, | |||
| "main-es2015": 266648, | |||
| "main-es2015": 267182, | |||
There was a problem hiding this comment.
@kara fyi - this regresses on cli-hello-world-lazy by 267182-266869=313 bytes
I don't see an obvious way to cut this back, and since this code is going to be significantly overhauled by misko's PRs let's revisit what the size decrease looks like after his refactor.
IgorMinar
added
the
merge: caretaker note
|
merge-assistance: global approval |
…changes (#34307) Prior to this change, in Ivy mode ngStyle/ngClass would accidentally emit value changes for static (string-based) values even if the value itself had not changed. This patch ensures that the style/class diffing code is more strict and when it signals ngClass/ngStyle that there has been a value change. Fixes #34336, #34444 PR Close #34307
Since I was learning the codebase and had a hard time understanding what was going on I've done a bunch of changes in one commit that under normal circumstances should have been split into several commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to spend the time with trying to split this up. Overall I've done the following: - I processed review feedback from #34307 - I did a bunch of renaming to make the code easier to understand - I refactored some internal functions that were either inefficient or hard to read - I also updated lots of type signatures to correct them and to remove many casts in the code PR Close #34307
Since I was learning the codebase and had a hard time understanding what was going on I've done a bunch of changes in one commit that under normal circumstances should have been split into several commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to spend the time with trying to split this up. Overall I've done the following: - I processed review feedback from #34307 - I did a bunch of renaming to make the code easier to understand - I refactored some internal functions that were either inefficient or hard to read - I also updated lots of type signatures to correct them and to remove many casts in the code PR Close #34307
|
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. |
Prior to this patch, ngStyle/ngClass would accidentally emit value
changes for static (string-based) values even if the value itself had
not changed. This patch ensures that the style/class diffing code is
more strict and when it signals ngClass/ngStyle that there has been a
value change.