Skip to content

Commit 6b8e60c

Browse files
cexbrayatthePunderWoman
authored andcommittedJul 18, 2022
fix(compiler-cli): improve the missingControlFlowDirective message (#46846)
The extended diagnostics about missing control flow directive was only mentioning that the `CommonModule` should be imported. Now that the control flow directives are available as standalone, the message mentions that directive itself can be imported. The message now also mentions which import should be used for the directive (as it can be tricky to figure out that `NgForOf` is the directive corresponding to `*ngFor`). PR Close #46846
1 parent 5004b6e commit 6b8e60c

File tree

2 files changed

+24
-9
lines changed

2 files changed

+24
-9
lines changed
 

‎packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive/index.ts

+14-5
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@ import {NgTemplateDiagnostic} from '../../../api';
1515
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
1616

1717
/**
18-
* The list of known control flow directives present in the `CommonModule`.
18+
* The list of known control flow directives present in the `CommonModule`,
19+
* and their corresponding imports.
1920
*
2021
* Note: there is no `ngSwitch` here since it's typically used as a regular
2122
* binding (e.g. `[ngSwitch]`), however the `ngSwitchCase` and `ngSwitchDefault`
2223
* are used as structural directives and a warning would be generated. Once the
2324
* `CommonModule` is included, the `ngSwitch` would also be covered.
2425
*/
25-
const KNOWN_CONTROL_FLOW_DIRECTIVES = new Set(['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault']);
26+
export const KNOWN_CONTROL_FLOW_DIRECTIVES = new Map([
27+
['ngIf', 'NgIf'], ['ngFor', 'NgForOf'], ['ngSwitchCase', 'NgSwitchCase'],
28+
['ngSwitchDefault', 'NgSwitchDefault']
29+
]);
2630

2731
/**
2832
* Ensures that there are no known control flow directives (such as *ngIf and *ngFor)
@@ -64,9 +68,14 @@ class MissingControlFlowDirectiveCheck extends
6468
}
6569

6670
const sourceSpan = controlFlowAttr.keySpan || controlFlowAttr.sourceSpan;
67-
const errorMessage = `The \`*${controlFlowAttr.name}\` directive was used in the template, ` +
68-
`but the \`CommonModule\` was not imported. Please make sure that the \`CommonModule\` ` +
69-
`is included into the \`@Component.imports\` array of this component.`;
71+
const correspondingImport = KNOWN_CONTROL_FLOW_DIRECTIVES.get(controlFlowAttr.name);
72+
const errorMessage =
73+
`The \`*${controlFlowAttr.name}\` directive was used in the template, ` +
74+
`but neither the \`${
75+
correspondingImport}\` directive nor the \`CommonModule\` was imported. ` +
76+
`Please make sure that either the \`${
77+
correspondingImport}\` directive or the \`CommonModule\` ` +
78+
`is included in the \`@Component.imports\` array of this component.`;
7079
const diagnostic = ctx.makeTemplateDiagnostic(sourceSpan, errorMessage);
7180
return [diagnostic];
7281
}

‎packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_control_flow_directive/missing_control_flow_directive_spec.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@ import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
1313
import {runInEachFileSystem} from '../../../../../file_system/testing';
1414
import {getSourceCodeForDiagnostic} from '../../../../../testing';
1515
import {getClass, setup} from '../../../../testing';
16-
import {factory as missingControlFlowDirectiveCheck} from '../../../checks/missing_control_flow_directive';
16+
import {factory as missingControlFlowDirectiveCheck, KNOWN_CONTROL_FLOW_DIRECTIVES} from '../../../checks/missing_control_flow_directive';
1717
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';
1818

19-
const KNOWN_CONTROL_FLOW_DIRECTIVES = ['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault'];
20-
2119
runInEachFileSystem(() => {
2220
describe('MissingControlFlowDirectiveCheck', () => {
23-
KNOWN_CONTROL_FLOW_DIRECTIVES.forEach(directive => {
21+
KNOWN_CONTROL_FLOW_DIRECTIVES.forEach((correspondingImport, directive) => {
2422
['div', 'ng-template', 'ng-container', 'ng-content'].forEach(element => {
2523
it(`should produce a warning when the '${directive}' directive is not imported ` +
2624
`(when used on the '${element}' element)`,
@@ -47,6 +45,14 @@ runInEachFileSystem(() => {
4745
expect(diags.length).toBe(1);
4846
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
4947
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_CONTROL_FLOW_DIRECTIVE));
48+
expect(diags[0].messageText)
49+
.toBe(
50+
`The \`*${directive}\` directive was used in the template, ` +
51+
`but neither the \`${
52+
correspondingImport}\` directive nor the \`CommonModule\` was imported. ` +
53+
`Please make sure that either the \`${
54+
correspondingImport}\` directive or the \`CommonModule\` ` +
55+
`is included in the \`@Component.imports\` array of this component.`);
5056
expect(getSourceCodeForDiagnostic(diags[0])).toBe(directive);
5157
});
5258

0 commit comments

Comments
 (0)
Please sign in to comment.