fix(compiler-cli): missingStructuralDirective diagnostic produces false negatives #64470
Conversation
| expect(diags.length).toBe(0); | ||
| }); | ||
|
|
||
| it('should *not* produce a warning for ngIf with template references using then/else', () => { |
There was a problem hiding this comment.
Found this fix was needed while testing this patch on our repo
| type: 'directive', | ||
| name: 'BarDirective', | ||
| selector: '[bar]', | ||
| inputs: {bar: 'bar'}, |
There was a problem hiding this comment.
Have you looked into why the test was passing when this specific input was removed ? (without the fix in).
There was a problem hiding this comment.
The diagnostic doesn't even run without it - I presume it results in a template compilation error which prevents the diagnostic from running (which we don't check for in the test)
84292cb to
ca36ffa
Compare
| { | ||
| type: 'directive', | ||
| name: 'BarDirective', | ||
| selector: 'ng-template[bar]', |
There was a problem hiding this comment.
This was another issue found while testing - *cdkDragPreview matches on ng-template[cdkDragPreview]
ca36ffa to
6d6b6cc
Compare
…uces false negatives (angular#64470)" This reverts commit c1d870b.
…uces false negatives (angular#64470)" This reverts commit 371274b.
|
We had to revert it, it was breaking some projects inside G3. |
|
We had a rebase issue due to the revert, the change is now at #64579 |
|
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
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #64467
What is the new behavior?
Fixes a bug in the missingStructuralDirective diagnostic where structural directives with missing imports were not reported when the element using the structural directive contained other directives
Does this PR introduce a breaking change?
I guess if this reports new issues then it would be counted as a breaking change? Maybe it should only be landed on v21 to be safe?
Other information