Skip to content

Commit

Permalink
fix(core): improve the missing control flow directive message
Browse files Browse the repository at this point in the history
Similarly to what has been done in angular#46846 for the extended diagnostics about missing control flow directive that was only mentioning that the `CommonModule` should be imported, this commit improves the validation done by the JiT compiler.
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 20, 2022
1 parent 0dc754a commit 08219e0
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
15 changes: 9 additions & 6 deletions packages/core/src/render3/instructions/element_validation.ts
Expand Up @@ -187,9 +187,11 @@ export function handleUnknownPropertyError(
'a part of an @NgModule where this component is declared';
if (KNOWN_CONTROL_FLOW_DIRECTIVES.has(propName)) {
// Most likely this is a control flow directive (such as `*ngIf`) used in
// a template, but the `CommonModule` is not imported.
// a template, but the directive or the `CommonModule` is not imported.
const correspondingImport = KNOWN_CONTROL_FLOW_DIRECTIVES.get(propName);
message += `\nIf the '${propName}' is an Angular control flow directive, ` +
`please make sure that the 'CommonModule' is ${importLocation}.`;
`please make sure that either the '${
correspondingImport}' directive or the 'CommonModule' is ${importLocation}.`;
} else {
// May be an Angular component, which is not imported/declared?
message += `\n1. If '${tagName}' is an Angular component and it has the ` +
Expand Down Expand Up @@ -275,13 +277,14 @@ function getTemplateLocationDetails(lView: LView): string {
}

/**
* The set of known control flow directives.
* The set of known control flow directives and their corresponding imports.
* We use this set to produce a more precises error message with a note
* that the `CommonModule` should also be included.
*/
export const KNOWN_CONTROL_FLOW_DIRECTIVES =
new Set(['ngIf', 'ngFor', 'ngSwitch', 'ngSwitchCase', 'ngSwitchDefault']);

export const KNOWN_CONTROL_FLOW_DIRECTIVES = new Map([
['ngIf', 'NgIf'], ['ngFor', 'NgForOf'], ['ngSwitchCase', 'NgSwitchCase'],
['ngSwitchDefault', 'NgSwitchDefault']
]);
/**
* Returns true if the tag name is allowed by specified schemas.
* @param schemas Array of schemas
Expand Down
8 changes: 5 additions & 3 deletions packages/core/test/acceptance/ng_module_spec.ts
Expand Up @@ -634,7 +634,7 @@ describe('NgModule', () => {
lines.forEach(line => expect(errorMessage).toMatch(line));
});

KNOWN_CONTROL_FLOW_DIRECTIVES.forEach(directive => {
KNOWN_CONTROL_FLOW_DIRECTIVES.forEach((correspondingImport, directive) => {
it(`should produce a warning when the '${directive}' directive ` +
`is used in a template, but not imported in corresponding NgModule`,
() => {
Expand Down Expand Up @@ -663,7 +663,8 @@ describe('NgModule', () => {
`NG0303: Can't bind to '${
directive}' since it isn't a known property of 'div' \\(used in the 'App' component template\\).`,
`If the '${directive}' is an Angular control flow directive, please make sure ` +
`that the 'CommonModule' is a part of an @NgModule where this component is declared.`
`that either the '${
correspondingImport}' directive or the 'CommonModule' is a part of an @NgModule where this component is declared.`
];
lines.forEach(line => expect(errorMessage).toMatch(line));
});
Expand All @@ -690,7 +691,8 @@ describe('NgModule', () => {
`NG0303: Can't bind to '${
directive}' since it isn't a known property of 'div' \\(used in the 'App' component template\\).`,
`If the '${directive}' is an Angular control flow directive, please make sure ` +
`that the 'CommonModule' is included in the '@Component.imports' of this component.`
`that either the '${
correspondingImport}' directive or the 'CommonModule' is included in the '@Component.imports' of this component.`
];
lines.forEach(line => expect(errorMessage).toMatch(line));
});
Expand Down

0 comments on commit 08219e0

Please sign in to comment.