Skip to content

Commit e620b3a

Browse files
crisbetodylhunn
crisbeto
authored and
dylhunn
committed
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
1 parent 8195be1 commit e620b3a

File tree

10 files changed

+130
-16
lines changed

10 files changed

+130
-16
lines changed

‎goldens/public-api/compiler-cli/extended_template_diagnostic_name.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
// @public
88
export enum ExtendedTemplateDiagnosticName {
9+
// (undocumented)
10+
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = "controlFlowPreventingContentProjection",
911
// (undocumented)
1012
INTERPOLATED_SIGNAL_NOT_INVOKED = "interpolatedSignalNotInvoked",
1113
// (undocumented)

‎packages/compiler-cli/src/ngtsc/core/src/compiler.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {StandaloneComponentScopeReader} from '../../scope/src/standalone';
3131
import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform';
3232
import {TemplateTypeCheckerImpl} from '../../typecheck';
3333
import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api';
34-
import {ALL_DIAGNOSTIC_FACTORIES, ExtendedTemplateCheckerImpl} from '../../typecheck/extended';
34+
import {ALL_DIAGNOSTIC_FACTORIES, ExtendedTemplateCheckerImpl, SUPPORTED_DIAGNOSTIC_NAMES} from '../../typecheck/extended';
3535
import {ExtendedTemplateChecker} from '../../typecheck/extended/api';
3636
import {getSourceFileOrNull, isDtsPath, toUnredirectedSourceFile} from '../../util/src/typescript';
3737
import {Xi18nContext} from '../../xi18n';
@@ -782,6 +782,8 @@ export class NgCompiler {
782782
// (providing the full TemplateTypeChecker API) and if strict mode is not enabled. In strict
783783
// mode, the user is in full control of type inference.
784784
suggestionsForSuboptimalTypeInference: this.enableTemplateTypeChecker && !strictTemplates,
785+
controlFlowPreventingContentProjection:
786+
this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
785787
};
786788
} else {
787789
typeCheckingConfig = {
@@ -810,6 +812,8 @@ export class NgCompiler {
810812
// In "basic" template type-checking mode, no warnings are produced since most things are
811813
// not checked anyways.
812814
suggestionsForSuboptimalTypeInference: false,
815+
controlFlowPreventingContentProjection:
816+
this.options.extendedDiagnostics?.defaultCategory || DiagnosticCategoryLabel.Warning,
813817
};
814818
}
815819

@@ -848,6 +852,11 @@ export class NgCompiler {
848852
if (this.options.strictLiteralTypes !== undefined) {
849853
typeCheckingConfig.strictLiteralTypes = this.options.strictLiteralTypes;
850854
}
855+
if (this.options.extendedDiagnostics?.checks?.controlFlowPreventingContentProjection !==
856+
undefined) {
857+
typeCheckingConfig.controlFlowPreventingContentProjection =
858+
this.options.extendedDiagnostics.checks.controlFlowPreventingContentProjection;
859+
}
851860

852861
return typeCheckingConfig;
853862
}
@@ -1285,18 +1294,16 @@ ${allowedCategoryLabels.join('\n')}
12851294
});
12861295
}
12871296

1288-
const allExtendedDiagnosticNames =
1289-
ALL_DIAGNOSTIC_FACTORIES.map((factory) => factory.name) as string[];
12901297
for (const [checkName, category] of Object.entries(options.extendedDiagnostics?.checks ?? {})) {
1291-
if (!allExtendedDiagnosticNames.includes(checkName)) {
1298+
if (!SUPPORTED_DIAGNOSTIC_NAMES.has(checkName)) {
12921299
yield makeConfigDiagnostic({
12931300
category: ts.DiagnosticCategory.Error,
12941301
code: ErrorCode.CONFIG_EXTENDED_DIAGNOSTICS_UNKNOWN_CHECK,
12951302
messageText: `
12961303
Angular compiler option "extendedDiagnostics.checks" has an unknown check: "${checkName}".
12971304
12981305
Allowed check names are:
1299-
${allExtendedDiagnosticNames.join('\n')}
1306+
${Array.from(SUPPORTED_DIAGNOSTIC_NAMES).join('\n')}
13001307
`.trim(),
13011308
});
13021309
}

‎packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ export enum ExtendedTemplateDiagnosticName {
2424
MISSING_NGFOROF_LET = 'missingNgForOfLet',
2525
SUFFIX_NOT_SUPPORTED = 'suffixNotSupported',
2626
SKIP_HYDRATION_NOT_STATIC = 'skipHydrationNotStatic',
27-
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked'
27+
INTERPOLATED_SIGNAL_NOT_INVOKED = 'interpolatedSignalNotInvoked',
28+
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 'controlFlowPreventingContentProjection',
2829
}

‎packages/compiler-cli/src/ngtsc/typecheck/api/api.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,11 @@ export interface TypeCheckingConfig {
278278
*/
279279
checkQueries: false;
280280

281+
/**
282+
* Whether to check if control flow syntax will prevent a node from being projected.
283+
*/
284+
controlFlowPreventingContentProjection: 'error'|'warning'|'suppress';
285+
281286
/**
282287
* Whether to use any generic types of the context component.
283288
*

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,9 @@ export const ALL_DIAGNOSTIC_FACTORIES:
2727
textAttributeNotBindingFactory, missingNgForOfLetFactory, suffixNotSupportedFactory,
2828
interpolatedSignalNotInvoked
2929
];
30+
31+
32+
export const SUPPORTED_DIAGNOSTIC_NAMES = new Set<string>([
33+
ExtendedTemplateDiagnosticName.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION,
34+
...ALL_DIAGNOSTIC_FACTORIES.map(factory => factory.name)
35+
]);

‎packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,9 @@ export interface OutOfBandDiagnosticRecorder {
105105
* Reports cases where control flow nodes prevent content projection.
106106
*/
107107
controlFlowPreventingContentProjection(
108-
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
109-
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
108+
templateId: TemplateId, category: ts.DiagnosticCategory,
109+
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
110+
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
110111
preservesWhitespaces: boolean): void;
111112
}
112113

@@ -350,8 +351,9 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
350351
}
351352

352353
controlFlowPreventingContentProjection(
353-
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
354-
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
354+
templateId: TemplateId, category: ts.DiagnosticCategory,
355+
projectionNode: TmplAstElement|TmplAstTemplate, componentName: string, slotSelector: string,
356+
controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
355357
preservesWhitespaces: boolean): void {
356358
const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
357359
const lines = [
@@ -367,14 +369,19 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
367369

368370
if (preservesWhitespaces) {
369371
lines.push(
370-
`Note: the host component has \`preserveWhitespaces: true\` which may ` +
371-
`cause whitespace to affect content projection.`);
372+
'Note: the host component has `preserveWhitespaces: true` which may ' +
373+
'cause whitespace to affect content projection.');
372374
}
373375

376+
lines.push(
377+
'',
378+
'This check can be disabled using the `extendedDiagnostics.checks.' +
379+
'controlFlowPreventingContentProjection = "suppress" compiler option.`');
380+
374381
this._diagnostics.push(makeTemplateDiagnostic(
375382
templateId, this.resolver.getSourceMapping(templateId), projectionNode.startSourceSpan,
376-
ts.DiagnosticCategory.Warning,
377-
ngErrorCode(ErrorCode.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION), lines.join('\n')));
383+
category, ngErrorCode(ErrorCode.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION),
384+
lines.join('\n')));
378385
}
379386
}
380387

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -918,10 +918,18 @@ class TcbDomSchemaCheckerOp extends TcbOp {
918918
* flow node didn't exist.
919919
*/
920920
class TcbControlFlowContentProjectionOp extends TcbOp {
921+
private readonly category: ts.DiagnosticCategory;
922+
921923
constructor(
922924
private tcb: Context, private element: TmplAstElement, private ngContentSelectors: string[],
923925
private componentName: string) {
924926
super();
927+
928+
// We only need to account for `error` and `warning` since
929+
// this check won't be enabled for `suppress`.
930+
this.category = tcb.env.config.controlFlowPreventingContentProjection === 'error' ?
931+
ts.DiagnosticCategory.Error :
932+
ts.DiagnosticCategory.Warning;
925933
}
926934

927935
override readonly optional = false;
@@ -944,7 +952,7 @@ class TcbControlFlowContentProjectionOp extends TcbOp {
944952
if (child instanceof TmplAstElement || child instanceof TmplAstTemplate) {
945953
matcher.match(createCssSelectorFromNode(child), (_, originalSelector) => {
946954
this.tcb.oobRecorder.controlFlowPreventingContentProjection(
947-
this.tcb.id, child, this.componentName, originalSelector, root,
955+
this.tcb.id, this.category, child, this.componentName, originalSelector, root,
948956
this.tcb.hostPreserveWhitespaces);
949957
});
950958
}
@@ -1940,7 +1948,9 @@ class Scope {
19401948
if (node instanceof TmplAstElement) {
19411949
const opIndex = this.opQueue.push(new TcbElementOp(this.tcb, this, node)) - 1;
19421950
this.elementOpMap.set(node, opIndex);
1943-
this.appendContentProjectionCheckOp(node);
1951+
if (this.tcb.env.config.controlFlowPreventingContentProjection !== 'suppress') {
1952+
this.appendContentProjectionCheckOp(node);
1953+
}
19441954
this.appendDirectivesAndInputsOfNode(node);
19451955
this.appendOutputsOfNode(node);
19461956
this.appendChildren(node);

‎packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,7 @@ describe('type check blocks', () => {
787787
enableTemplateTypeChecker: false,
788788
useInlineTypeConstructors: true,
789789
suggestionsForSuboptimalTypeInference: false,
790+
controlFlowPreventingContentProjection: 'warning',
790791
};
791792

792793
describe('config.applyTemplateContextGuards', () => {

‎packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ export const ALL_ENABLED_CONFIG: Readonly<TypeCheckingConfig> = {
220220
enableTemplateTypeChecker: false,
221221
useInlineTypeConstructors: true,
222222
suggestionsForSuboptimalTypeInference: false,
223+
controlFlowPreventingContentProjection: 'warning',
223224
};
224225

225226
// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
@@ -323,6 +324,7 @@ export function tcb(
323324
checkTypeOfPipes: true,
324325
checkTemplateBodies: true,
325326
alwaysCheckSchemaInTemplateBodies: true,
327+
controlFlowPreventingContentProjection: 'warning',
326328
strictSafeNavigationTypes: true,
327329
useContextGenericType: true,
328330
strictLiteralTypes: true,

‎packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5352,6 +5352,79 @@ suppress
53525352
const diags = env.driveDiagnostics();
53535353
expect(diags.length).toBe(0);
53545354
});
5355+
5356+
it('should allow the content projection diagnostic to be disabled individually', () => {
5357+
env.tsconfig({
5358+
extendedDiagnostics: {
5359+
checks: {
5360+
controlFlowPreventingContentProjection: DiagnosticCategoryLabel.Suppress,
5361+
}
5362+
}
5363+
});
5364+
env.write('test.ts', `
5365+
import {Component} from '@angular/core';
5366+
5367+
@Component({
5368+
selector: 'comp',
5369+
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
5370+
standalone: true,
5371+
})
5372+
class Comp {}
5373+
5374+
@Component({
5375+
standalone: true,
5376+
imports: [Comp],
5377+
template: \`
5378+
<comp>
5379+
@if (true) {
5380+
<div foo></div>
5381+
breaks projection
5382+
}
5383+
</comp>
5384+
\`,
5385+
})
5386+
class TestCmp {}
5387+
`);
5388+
5389+
const diags = env.driveDiagnostics();
5390+
expect(diags.length).toBe(0);
5391+
});
5392+
5393+
it('should allow the content projection diagnostic to be disabled via `defaultCategory`',
5394+
() => {
5395+
env.tsconfig({
5396+
extendedDiagnostics: {
5397+
defaultCategory: DiagnosticCategoryLabel.Suppress,
5398+
}
5399+
});
5400+
env.write('test.ts', `
5401+
import {Component} from '@angular/core';
5402+
5403+
@Component({
5404+
selector: 'comp',
5405+
template: '<ng-content/> <ng-content select="bar, [foo]"/>',
5406+
standalone: true,
5407+
})
5408+
class Comp {}
5409+
5410+
@Component({
5411+
standalone: true,
5412+
imports: [Comp],
5413+
template: \`
5414+
<comp>
5415+
@if (true) {
5416+
<div foo></div>
5417+
breaks projection
5418+
}
5419+
</comp>
5420+
\`,
5421+
})
5422+
class TestCmp {}
5423+
`);
5424+
5425+
const diags = env.driveDiagnostics();
5426+
expect(diags.length).toBe(0);
5427+
});
53555428
});
53565429
});
53575430
});

0 commit comments

Comments
 (0)