fix(compiler-cli): add compiler option to disable control flow content projection diagnostic (#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from #53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close #53311
This commit is contained in:
Kristiyan Kostadinov 2023-12-01 13:53:22 +01:00 committed by Dylan Hunn
parent 8195be1e09
commit e620b3a724
10 changed files with 130 additions and 16 deletions

View file

@ -6,6 +6,8 @@
// @public
export enum ExtendedTemplateDiagnosticName {
// (undocumented)
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = "controlFlowPreventingContentProjection",
// (undocumented)
INTERPOLATED_SIGNAL_NOT_INVOKED = "interpolatedSignalNotInvoked",
// (undocumented)

View file

@ -31,7 +31,7 @@ import {StandaloneComponentScopeReader} from '../../scope/src/standalone';
import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform';
import {TemplateTypeCheckerImpl} from '../../typecheck';
import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api';
import {ALL_DIAGNOSTIC_FACTORIES, ExtendedTemplateCheckerImpl} from '../../typecheck/extended';
import {ALL_DIAGNOSTIC_FACTORIES, ExtendedTemplateCheckerImpl, SUPPORTED_DIAGNOSTIC_NAMES} from '../../typecheck/extended';
import {ExtendedTemplateChecker} from '../../typecheck/extended/api';
import {getSourceFileOrNull, isDtsPath, toUnredirectedSourceFile} from '../../util/src/typescript';
import {Xi18nContext} from '../../xi18n';
@ -782,6 +782,8 @@ export class NgCompiler {
// (providing the full TemplateTypeChecker API) and if strict mode is not enabled. In strict
// mode, the user is in full control of type inference.
suggestionsForSuboptimalTypeInference: this.enableTemplateTypeChecker && !strictTemplates,
controlFlowPreventingContentProjection:
this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
};
} else {
typeCheckingConfig = {
@ -810,6 +812,8 @@ export class NgCompiler {
// In "basic" template type-checking mode, no warnings are produced since most things are
// not checked anyways.
suggestionsForSuboptimalTypeInference: false,
controlFlowPreventingContentProjection:
this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
};
}
@ -848,6 +852,11 @@ export class NgCompiler {
if (this.options.strictLiteralTypes !== undefined) {
typeCheckingConfig.strictLiteralTypes = this.options.strictLiteralTypes;
}
if (this.options.extendedDiagnostics?.checks?.controlFlowPreventingContentProjection !==
undefined) {
typeCheckingConfig.controlFlowPreventingContentProjection =
this.options.extendedDiagnostics.checks.controlFlowPreventingContentProjection;
}
return typeCheckingConfig;
}
@ -1285,10 +1294,8 @@ ${allowedCategoryLabels.join('\n')}
});
}
const allExtendedDiagnosticNames =
ALL_DIAGNOSTIC_FACTORIES.map((factory) => factory.name) as string[];
for (const [checkName, category] of Object.entries(options.extendedDiagnostics?.checks ?? {})) {
if (!allExtendedDiagnosticNames.includes(checkName)) {
if (!SUPPORTED_DIAGNOSTIC_NAMES.has(checkName)) {
yield makeConfigDiagnostic({
category: ts.DiagnosticCategory.Error,
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CHECK,
@ -1296,7 +1303,7 @@ ${allowedCategoryLabels.join('\n')}
Angular compiler option "extendedDiagnostics.checks" has an unknown check: "${checkName}".
Allowed check names are:
${allExtendedDiagnosticNames.join('\n')}
${Array.from(SUPPORTED_DIAGNOSTIC_NAMES).join('\n')}
`.trim(),
});
}

View file

@ -24,5 +24,6 @@ export enum ExtendedTemplateDiagnosticName {
MISSING_NGFOROF_LET = 'missingNgForOfLet',
SUFFIX_NOT_SUPPORTED = 'suffixNotSupported',
SKIP_HYDRATION_NOT_STATIC = 'skipHydrationNotStatic',
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked'
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked',
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 'controlFlowPreventingContentProjection',
}

View file

@ -278,6 +278,11 @@ export interface TypeCheckingConfig {
*/
checkQueries: false;
/**
* Whether to check if control flow syntax will prevent a node from being projected.
*/
controlFlowPreventingContentProjection: 'error'|'warning'|'suppress';
/**
* Whether to use any generic types of the context component.
*

View file

@ -27,3 +27,9 @@ export const ALL_DIAGNOSTIC_FACTORIES:
textAttributeNotBindingFactory, missingNgForOfLetFactory, suffixNotSupportedFactory,
interpolatedSignalNotInvoked
];
export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
ExtendedTemplateDiagnosticName.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION,
...ALL_DIAGNOSTIC_FACTORIES.map(factory => factory.name)
]);

View file

@ -105,8 +105,9 @@ export interface OutOfBandDiagnosticRecorder {
* Reports cases where control flow nodes prevent content projection.
*/
controlFlowPreventingContentProjection(
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
preservesWhitespaces: boolean): void;
}
@ -350,8 +351,9 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
}
controlFlowPreventingContentProjection(
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
templateId: TemplateId, category: ts.DiagnosticCategory,
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
preservesWhitespaces: boolean): void {
const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
const lines = [
@ -367,14 +369,19 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
if (preservesWhitespaces) {
lines.push(
`Note: the host component has \`preserveWhitespaces: true\` which may ` +
`cause whitespace to affect content projection.`);
'Note: the host component has `preserveWhitespaces: true` which may ' +
'cause whitespace to affect content projection.');
}
lines.push(
'',
'This check can be disabled using the `extendedDiagnostics.checks.' +
'controlFlowPreventingContentProjection = "suppress" compiler option.`');
this._diagnostics.push(makeTemplateDiagnostic(
templateId, this.resolver.getSourceMapping(templateId), projectionNode.startSourceSpan,
ts.DiagnosticCategory.Warning,
ngErrorCode(ErrorCode.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION), lines.join('\n')));
category, ngErrorCode(ErrorCode.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION),
lines.join('\n')));
}
}

View file

@ -918,10 +918,18 @@ class TcbDomSchemaCheckerOp extends TcbOp {
* flow node didn't exist.
*/
class TcbControlFlowContentProjectionOp extends TcbOp {
private readonly category: ts.DiagnosticCategory;
constructor(
private tcb: Context, private element: TmplAstElement, private ngContentSelectors: string[],
private componentName: string) {
super();
// We only need to account for `error` and `warning` since
// this check won't be enabled for `suppress`.
this.category = tcb.env.config.controlFlowPreventingContentProjection === 'error' ?
ts.DiagnosticCategory.Error :
ts.DiagnosticCategory.Warning;
}
override readonly optional = false;
@ -944,7 +952,7 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
if (child instanceof TmplAstElement || child instanceof TmplAstTemplate) {
matcher.match(createCssSelectorFromNode(child), (_, originalSelector) => {
this.tcb.oobRecorder.controlFlowPreventingContentProjection(
this.tcb.id, child, this.componentName, originalSelector, root,
this.tcb.id, this.category, child, this.componentName, originalSelector, root,
this.tcb.hostPreserveWhitespaces);
});
}
@ -1940,7 +1948,9 @@ class Scope {
if (node instanceof TmplAstElement) {
const opIndex = this.opQueue.push(new TcbElementOp(this.tcb, this, node)) - 1;
this.elementOpMap.set(node, opIndex);
this.appendContentProjectionCheckOp(node);
if (this.tcb.env.config.controlFlowPreventingContentProjection !== 'suppress') {
this.appendContentProjectionCheckOp(node);
}
this.appendDirectivesAndInputsOfNode(node);
this.appendOutputsOfNode(node);
this.appendChildren(node);

View file

@ -787,6 +787,7 @@ describe('type check blocks', () => {
enableTemplateTypeChecker: false,
useInlineTypeConstructors: true,
suggestionsForSuboptimalTypeInference: false,
controlFlowPreventingContentProjection: 'warning',
};
describe('config.applyTemplateContextGuards', () => {

View file

@ -220,6 +220,7 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
enableTemplateTypeChecker: false,
useInlineTypeConstructors: true,
suggestionsForSuboptimalTypeInference: false,
controlFlowPreventingContentProjection: 'warning',
};
// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
@ -323,6 +324,7 @@ export function tcb(
checkTypeOfPipes: true,
checkTemplateBodies: true,
alwaysCheckSchemaInTemplateBodies: true,
controlFlowPreventingContentProjection: 'warning',
strictSafeNavigationTypes: true,
useContextGenericType: true,
strictLiteralTypes: true,

View file

@ -5352,6 +5352,79 @@ suppress
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should allow the content projection diagnostic to be disabled individually', () => {
env.tsconfig({
extendedDiagnostics: {
checks: {
controlFlowPreventingContentProjection: DiagnosticCategoryLabel.Suppress,
}
}
});
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp',
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
standalone: true,
})
class Comp {}
@Component({
standalone: true,
imports: [Comp],
template: \`
<comp>
@if (true) {
<div foo></div>
breaks projection
}
</comp>
\`,
})
class TestCmp {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should allow the content projection diagnostic to be disabled via `defaultCategory`',
() => {
env.tsconfig({
extendedDiagnostics: {
defaultCategory: DiagnosticCategoryLabel.Suppress,
}
});
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'comp',
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
standalone: true,
})
class Comp {}
@Component({
standalone: true,
imports: [Comp],
template: \`
<comp>
@if (true) {
<div foo></div>
breaks projection
}
</comp>
\`,
})
class TestCmp {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
});
});
});