strictTemplates implies fullTemplateTypeCheck#34195
strictTemplates implies fullTemplateTypeCheck#34195JoostK wants to merge 2 commits intoangular:masterfrom
strictTemplates implies fullTemplateTypeCheck#34195Conversation
There was a problem hiding this comment.
We could emit this as a diagnostic in getNgOptionDiagnostics. I've never done an option diagnostic though - might be worth a shot.
e08f2df to
9d1251e
Compare
IgorMinar
left a comment
There was a problem hiding this comment.
I suggest we clean up the docs a bit more. See individual commits.
thanks for making this change.
There was a problem hiding this comment.
I find this condition very misleading and confusing. I always have even before this change.
How about:
const templateTypeCheck = (strictTemplates || !!this.options.fullTemplateTypeCheck) && this.options.ivyTemplateTypeCheck !== false;
if (!templateTypeCheck) {
return [];
}
There was a problem hiding this comment.
You are not alone, it confuses me just as much as it confuses you :-)
Your suggestion, however, is not equivalent to the current logic. Currently, even if "ivyTemplateTypeCheck" is false will typechecking still be enabled if "fullTemplateTypeCheck"/"strictTemplates" is set. Your suggestion as I interpret it would not behave the same, as having "ivyTemplateTypeCheck" set to false would completely disable type-checking under all circumstances.
There was a problem hiding this comment.
I'm okay with the behavior change. If you're curious, the logic worked this way because fullTemplateTypeCheck is easy to control as an option of the ng_module build rule. So I could set ivyTemplateTypeCheck to false in bazel/g3, and then set fullTemplateTypeCheck via the ng_module option when I wanted to test type-checking in g3 projects.
This logic predates the strictTemplates though - now setting fullTemplateTypeCheck isn't enough. So I'm happy to change it.
There was a problem hiding this comment.
@alxhub Can we get rid of ivyTemplateTypeCheck entirely? non-fTTC should be at the same level asa it was in VE, so there's shouldn't be a special flag for disabling TTC in g3?
There was a problem hiding this comment.
I've been looking into what it would take to do this in g3, and there's a number of issues preventing it at the moment. So I think we should wait for now.
9d1251e to
d918d11
Compare
|
@IgorMinar thanks for the review, your suggestions have been applied. |
It is now an error if '"fullTemplateTypeCheck"' is disabled while `"strictTemplates"` is enabled, as enabling the latter implies that the former is also enabled.
Previously, it was required that both `fullTemplateTypeCheck` and `strictTemplates` had to be enabled for strict mode to be enabled. This is strange, as `strictTemplates` implies `fullTemplateTypeCheck`. This commit makes setting the `fullTemplateTypeCheck` flag optional so that strict mode can be enabled by just setting `strictTemplates`.
d918d11 to
25abc5a
Compare
…34195) Previously, it was required that both `fullTemplateTypeCheck` and `strictTemplates` had to be enabled for strict mode to be enabled. This is strange, as `strictTemplates` implies `fullTemplateTypeCheck`. This commit makes setting the `fullTemplateTypeCheck` flag optional so that strict mode can be enabled by just setting `strictTemplates`. PR Close #34195
) It is now an error if '"fullTemplateTypeCheck"' is disabled while `"strictTemplates"` is enabled, as enabling the latter implies that the former is also enabled. PR Close #34195
…34195) Previously, it was required that both `fullTemplateTypeCheck` and `strictTemplates` had to be enabled for strict mode to be enabled. This is strange, as `strictTemplates` implies `fullTemplateTypeCheck`. This commit makes setting the `fullTemplateTypeCheck` flag optional so that strict mode can be enabled by just setting `strictTemplates`. PR Close #34195
|
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. |
See individual commits.