Repository navigation
Conversation
097c8e5 to
5d844ce
Compare
There was a problem hiding this comment.
Would _switchValue be better with a default value? It's actually kind of correct with the ! right now. The directive won't match unless this is defined. It's a little unfortunate that we have to add code to account for the possibility of undefined when it really can't ever be in production.
There was a problem hiding this comment.
You're right, we can keep the type safety by passing the value to _updateView
There was a problem hiding this comment.
Should this have a default of empty array instead of undefined?
There was a problem hiding this comment.
I'd rather see undefined or null as a default value. It's created when needed by _addDefault and checked by _updateDefaultCases.
I'm always to other point of views though !
There was a problem hiding this comment.
Having it be nullable rather than empty list provides no additional meaning. I feel quite strongly that this should never be null or undefined. You can also update the rest of the code to remove the truthiness checks of this value.
There was a problem hiding this comment.
I used empty array as default value. It seems presubmit is failing now with the changes. Any heads-up ?
a6014c4 to
2478c77
Compare
2478c77 to
ea115c8
Compare
As part of angular#24571, removing all non-null assertions from common/directives
aa2afb1 to
b883f2a
Compare
atscott
left a comment
There was a problem hiding this comment.
@JeanMeche looks like the lint test is failing. The code changes otherwise look good and the internal test failures are unrelated
b883f2a to
4e03ca3
Compare
As part of angular#24571, removing all non-null assertions from common/directives
* removing guard as console.warn is now widely supported * Couldn't remove non-null assertion, waiting for TS support of getters with different types (microsoft/TypeScript#43662)
As part of angular#24571, removing all non-null assertions from common/test/directives
4e03ca3 to
51bd989
Compare
Done. |
|
This PR was merged into the repository by commit d887d69. |
* removing guard as console.warn is now widely supported * Couldn't remove non-null assertion, waiting for TS support of getters with different types (microsoft/TypeScript#43662) PR Close #48476
* removing guard as console.warn is now widely supported * Couldn't remove non-null assertion, waiting for TS support of getters with different types (microsoft/TypeScript#43662) PR Close #48476
* removing guard as console.warn is now widely supported * Couldn't remove non-null assertion, waiting for TS support of getters with different types (microsoft/TypeScript#43662) PR Close #48476
As part of angular#24571, removing all non-null assertions from common/directives PR Close angular#48476
As part of angular#24571, removing all non-null assertions from common/directives PR Close angular#48476
* removing guard as console.warn is now widely supported * Couldn't remove non-null assertion, waiting for TS support of getters with different types (microsoft/TypeScript#43662) PR Close angular#48476
As part of angular#24571, removing all non-null assertions from common/test/directives PR Close angular#48476
|
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. |
PR Checklist
Per request from @pkozlowski-opensource, I'm splitting PR #48241 into 2.
See individual commits.
These are fixes targeted to resolve #24571
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?