Skip to content

Commit

Permalink
fix(compiler-cli): improve the missingControlFlowDirective message
Browse files Browse the repository at this point in the history
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`).
  • Loading branch information
cexbrayat committed Jul 15, 2022
1 parent 62174b0 commit 8ad8cbe
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
Expand Up @@ -15,14 +15,18 @@ import {NgTemplateDiagnostic} from '../../../api';
import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';

/**
* The list of known control flow directives present in the `CommonModule`.
* The list of known control flow directives present in the `CommonModule`,
* and their corresponding imports.
*
* Note: there is no `ngSwitch` here since it's typically used as a regular
* binding (e.g. `[ngSwitch]`), however the `ngSwitchCase` and `ngSwitchDefault`
* are used as structural directives and a warning would be generated. Once the
* `CommonModule` is included, the `ngSwitch` would also be covered.
*/
const KNOWN_CONTROL_FLOW_DIRECTIVES = new Set(['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault']);
const KNOWN_CONTROL_FLOW_DIRECTIVES = new Map([
['ngIf', 'NgIf'], ['ngFor', 'NgForOf'], ['ngSwitchCase', 'NgSwitchCase'],
['ngSwitchDefault', 'NgSwitchDefault']
]);

/**
* Ensures that there are no known control flow directives (such as *ngIf and *ngFor)
Expand Down Expand Up @@ -64,9 +68,13 @@ class MissingControlFlowDirectiveCheck extends
}

const sourceSpan = controlFlowAttr.keySpan || controlFlowAttr.sourceSpan;
const errorMessage = `The \`*${controlFlowAttr.name}\` directive was used in the template, ` +
`but the \`CommonModule\` was not imported. Please make sure that the \`CommonModule\` ` +
`is included into the \`@Component.imports\` array of this component.`;
const correspondingImport = KNOWN_CONTROL_FLOW_DIRECTIVES.get(controlFlowAttr.name);
const errorMessage =
`The \`*${controlFlowAttr.name}\` directive was used in the template, ` +
`but the \`*${
correspondingImport}\` directive or the \`CommonModule\` were not imported. ` +
`Please make sure that the \`*${correspondingImport}\` directive or the \`CommonModule\` ` +
`are included in the \`@Component.imports\` array of this component.`;
const diagnostic = ctx.makeTemplateDiagnostic(sourceSpan, errorMessage);
return [diagnostic];
}
Expand Down
Expand Up @@ -17,10 +17,11 @@ import {factory as missingControlFlowDirectiveCheck} from '../../../checks/missi
import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker';

const KNOWN_CONTROL_FLOW_DIRECTIVES = ['ngIf', 'ngFor', 'ngSwitchCase', 'ngSwitchDefault'];
const CORRESPONDING_IMPORTS = ['NgIf', 'NgForOf', 'NgSwitchCase', 'NgSwitchDefault'];

runInEachFileSystem(() => {
describe('MissingControlFlowDirectiveCheck', () => {
KNOWN_CONTROL_FLOW_DIRECTIVES.forEach(directive => {
KNOWN_CONTROL_FLOW_DIRECTIVES.forEach((directive, index) => {
['div', 'ng-template', 'ng-container', 'ng-content'].forEach(element => {
it(`should produce a warning when the '${directive}' directive is not imported ` +
`(when used on the '${element}' element)`,
Expand All @@ -47,6 +48,15 @@ runInEachFileSystem(() => {
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_CONTROL_FLOW_DIRECTIVE));
const correspondingImport = CORRESPONDING_IMPORTS[index];
expect(diags[0].messageText)
.toBe(
`The \`*${directive}\` directive was used in the template, ` +
`but the \`*${
correspondingImport}\` directive or the \`CommonModule\` were not imported. ` +
`Please make sure that the \`*${
correspondingImport}\` directive or the \`CommonModule\` ` +
`are included in the \`@Component.imports\` array of this component.`);
expect(getSourceCodeForDiagnostic(diags[0])).toBe(directive);
});

Expand Down

0 comments on commit 8ad8cbe

Please sign in to comment.