feat(compiler-cli): detect missing control flow directive imports#46146
feat(compiler-cli): detect missing control flow directive imports#46146AndrewKushnir wants to merge 1 commit intoangular:mainfrom
Conversation
732c1c4 to
5ac5ecf
Compare
...ges/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts
Outdated
Show resolved
Hide resolved
5ac5ecf to
d5d83a4
Compare
|
Just wondering - but isn't this PR solving a specific case that should be solved by #37322 ? Once that one is fixed - ngIf without common module will be unknown structual directive like any other and error. |
|
@pkozlowski-opensource Shouldn't the error message (not from extended diagnostic) come from the compiler not finding the directive applied to the ng-emplate? I mean in both ngIf and any other structual directive the error is the same in case the directive isn't found (no error at all currently - Thus the linked issue). I agree that ngIf can be further extended to say the actual module that is missing. |
|
@Harpush right, suggestion about a NgModule to import is one aspect of it. When it comes to the "directive didn't match", the situation is more nuanced. #37322 talks about the specific situation: Now the question we should ask: what is the meaning of
As we see the (2) usage in the wild we don't have much choice but, at this point, assume the (2) meaning. This is why we can't error on seeing |
|
@pkozlowski-opensource I understand the problem... Isn't there a way to "remember" it was once |
|
@Harpush as I've mentioned we see usage of We could change the meaning to expect |
|
@pkozlowski-opensource I believe the current way some people use * is wrong... But it's indeed a breaking change and I am not sure it's a step you wish to take. A possible solution can be a compiler flag. We already have strict related compiler flags so why not add something like |
Not sure if there is right / wrong here. I think that we just never made it intentional (in teaching / supported syntax / errors) so people can see it both ways. At this point we are in the v14 RC period so a breaking change, even behind a compiler flag is not an option. And we would really need to dive deeper into desirable behaviour. Error message improvement is a temporary measure that we can do before the v14 final. |
d5d83a4 to
ad6701b
Compare
c1fb807 to
31ad317
Compare
...k/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts
Outdated
Show resolved
Hide resolved
31ad317 to
befd1bc
Compare
There was a problem hiding this comment.
reviewed-for: public-api
…standalone components This commit adds an extended diagnostics check that verifies that all control flow directives (such as `ngIf`, `ngFor`) have the necessary directives imported in standalone components. Currently there is no diagnostics produced for such cases, which makes it harder to detect and find problems.
befd1bc to
66bdbb1
Compare
|
This PR was merged into the repository by commit 131d029. |
|
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. |
This commit adds an extended diagnostics check that verifies that all control flow directives (such as
ngIf,ngFor) have matching directives. Currently there is no diagnistics produced for such cases, which makes it harder for developers to detect and find a problem.PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?