Skip to content

Commit f63c0f6

Browse files
crisbetoAndrewKushnir
crisbeto
authored and
AndrewKushnir
committed
Revert "fix(compiler-cli): add diagnostic for control flow that prevents content projection (#52726)" (#53012)
This reverts commit b4d022e. PR Close #53012
1 parent 6f67d0c commit f63c0f6

6 files changed

Lines changed: 2 additions & 465 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ export enum ErrorCode {
2525
// (undocumented)
2626
CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
2727
CONFLICTING_INPUT_TRANSFORM = 2020,
28-
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 8011,
2928
// (undocumented)
3029
DECORATOR_ARG_NOT_LITERAL = 1001,
3130
// (undocumented)

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -286,21 +286,6 @@ export enum ErrorCode {
286286
*/
287287
INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT = 8010,
288288

289-
/**
290-
* A control flow node is projected at the root of a component and is preventing its direct
291-
* descendants from being projected, because it has more than one root node.
292-
*
293-
* ```
294-
* <comp>
295-
* @if (expr) {
296-
* <div projectsIntoSlot></div>
297-
* Text preventing the div from being projected
298-
* }
299-
* </comp>
300-
* ```
301-
*/
302-
CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 8011,
303-
304289
/**
305290
* A two way binding in a template has an incorrect syntax,
306291
* parentheses outside brackets. For example:

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

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
9+
import {BindingPipe, PropertyRead, PropertyWrite, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstInteractionDeferredTrigger, TmplAstReference, TmplAstTemplate, TmplAstVariable, TmplAstViewportDeferredTrigger} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@@ -100,14 +100,6 @@ export interface OutOfBandDiagnosticRecorder {
100100
templateId: TemplateId,
101101
trigger: TmplAstHoverDeferredTrigger|TmplAstInteractionDeferredTrigger|
102102
TmplAstViewportDeferredTrigger): void;
103-
104-
/**
105-
* Reports cases where control flow nodes prevent content projection.
106-
*/
107-
controlFlowPreventingContentProjection(
108-
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
109-
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
110-
preservesWhitespaces: boolean): void;
111103
}
112104

113105
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
@@ -348,34 +340,6 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
348340
ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT),
349341
message));
350342
}
351-
352-
controlFlowPreventingContentProjection(
353-
templateId: TemplateId, projectionNode: TmplAstElement|TmplAstTemplate, componentName: string,
354-
slotSelector: string, controlFlowNode: TmplAstIfBlockBranch|TmplAstForLoopBlock,
355-
preservesWhitespaces: boolean): void {
356-
const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
357-
const lines = [
358-
`Node matches the "${slotSelector}" slot of the "${
359-
componentName}" component, but will not be projected into the specific slot because the surrounding ${
360-
blockName} has more than one node at its root. To project the node in the right slot, you can:\n`,
361-
`1. Wrap the content of the ${blockName} block in an <ng-container/> that matches the "${
362-
slotSelector}" selector.`,
363-
`2. Split the content of the ${blockName} block across multiple ${
364-
blockName} blocks such that each one only has a single projectable node at its root.`,
365-
`3. Remove all content from the ${blockName} block, except for the node being projected.`
366-
];
367-
368-
if (preservesWhitespaces) {
369-
lines.push(
370-
`Note: the host component has \`preserveWhitespaces: true\` which may ` +
371-
`cause whitespace to affect content projection.`);
372-
}
373-
374-
this._diagnostics.push(makeTemplateDiagnostic(
375-
templateId, this.resolver.getSourceMapping(templateId), projectionNode.startSourceSpan,
376-
ts.DiagnosticCategory.Warning,
377-
ngErrorCode(ErrorCode.CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION), lines.join('\n')));
378-
}
379343
}
380344

381345
function makeInlineDiagnostic(

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

Lines changed: 1 addition & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
9+
import {AST, BindingPipe, BindingType, BoundTarget, Call, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {Reference} from '../../imports';
@@ -905,100 +905,6 @@ class TcbDomSchemaCheckerOp extends TcbOp {
905905
}
906906

907907

908-
/**
909-
* A `TcbOp` that finds and flags control flow nodes that interfere with content projection.
910-
*
911-
* Context:
912-
* `@if` and `@for` try to emulate the content projection behavior of `*ngIf` and `*ngFor`
913-
* in order to reduce breakages when moving from one syntax to the other (see #52414), however the
914-
* approach only works if there's only one element at the root of the control flow expression.
915-
* This means that a stray sibling node (e.g. text) can prevent an element from being projected
916-
* into the right slot. The purpose of the `TcbOp` is to find any places where a node at the root
917-
* of a control flow expression *would have been projected* into a specific slot, if the control
918-
* flow node didn't exist.
919-
*/
920-
class TcbControlFlowContentProjectionOp extends TcbOp {
921-
constructor(
922-
private tcb: Context, private element: TmplAstElement, private ngContentSelectors: string[],
923-
private componentName: string) {
924-
super();
925-
}
926-
927-
override readonly optional = false;
928-
929-
override execute(): null {
930-
const controlFlowToCheck = this.findPotentialControlFlowNodes();
931-
932-
if (controlFlowToCheck.length > 0) {
933-
const matcher = new SelectorMatcher<string>();
934-
935-
for (const selector of this.ngContentSelectors) {
936-
// `*` is a special selector for the catch-all slot.
937-
if (selector !== '*') {
938-
matcher.addSelectables(CssSelector.parse(selector), selector);
939-
}
940-
}
941-
942-
for (const root of controlFlowToCheck) {
943-
for (const child of root.children) {
944-
if (child instanceof TmplAstElement || child instanceof TmplAstTemplate) {
945-
matcher.match(createCssSelectorFromNode(child), (_, originalSelector) => {
946-
this.tcb.oobRecorder.controlFlowPreventingContentProjection(
947-
this.tcb.id, child, this.componentName, originalSelector, root,
948-
this.tcb.hostPreserveWhitespaces);
949-
});
950-
}
951-
}
952-
}
953-
}
954-
955-
return null;
956-
}
957-
958-
private findPotentialControlFlowNodes() {
959-
const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock> = [];
960-
961-
for (const child of this.element.children) {
962-
let eligibleNode: TmplAstForLoopBlock|TmplAstIfBlockBranch|null = null;
963-
964-
// Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
965-
if (child instanceof TmplAstForLoopBlock) {
966-
eligibleNode = child;
967-
} else if (child instanceof TmplAstIfBlock) {
968-
eligibleNode = child.branches[0]; // @if blocks are guaranteed to have at least one branch.
969-
}
970-
971-
// Skip nodes with less than two children since it's impossible
972-
// for them to run into the issue that we're checking for.
973-
if (eligibleNode === null || eligibleNode.children.length < 2) {
974-
continue;
975-
}
976-
977-
// Count the number of root nodes while skipping empty text where relevant.
978-
const rootNodeCount = eligibleNode.children.reduce((count, node) => {
979-
// Normally `preserveWhitspaces` would have been accounted for during parsing, however
980-
// in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
981-
// `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
982-
// that we have to account for it here since the presence of text nodes affects the
983-
// content projection behavior.
984-
if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
985-
node.value.trim().length > 0) {
986-
count++;
987-
}
988-
989-
return count;
990-
}, 0);
991-
992-
// Content projection can only be affected if there is more than one root node.
993-
if (rootNodeCount > 1) {
994-
result.push(eligibleNode);
995-
}
996-
}
997-
998-
return result;
999-
}
1000-
}
1001-
1002908
/**
1003909
* Mapping between attributes names that don't correspond to their element property names.
1004910
* Note: this mapping has to be kept in sync with the equally named mapping in the runtime.
@@ -1931,7 +1837,6 @@ class Scope {
19311837
if (node instanceof TmplAstElement) {
19321838
const opIndex = this.opQueue.push(new TcbElementOp(this.tcb, this, node)) - 1;
19331839
this.elementOpMap.set(node, opIndex);
1934-
this.appendContentProjectionCheckOp(node);
19351840
this.appendDirectivesAndInputsOfNode(node);
19361841
this.appendOutputsOfNode(node);
19371842
this.appendChildren(node);
@@ -2128,22 +2033,6 @@ class Scope {
21282033
}
21292034
}
21302035

2131-
private appendContentProjectionCheckOp(root: TmplAstElement): void {
2132-
const meta =
2133-
this.tcb.boundTarget.getDirectivesOfNode(root)?.find(meta => meta.isComponent) || null;
2134-
2135-
if (meta !== null && meta.ngContentSelectors !== null && meta.ngContentSelectors.length > 0) {
2136-
const selectors = meta.ngContentSelectors;
2137-
2138-
// We don't need to generate anything for components that don't have projection
2139-
// slots, or they only have one catch-all slot (represented by `*`).
2140-
if (selectors.length > 1 || (selectors.length === 1 && selectors[0] !== '*')) {
2141-
this.opQueue.push(
2142-
new TcbControlFlowContentProjectionOp(this.tcb, root, selectors, meta.name));
2143-
}
2144-
}
2145-
}
2146-
21472036
private appendDeferredBlock(block: TmplAstDeferredBlock): void {
21482037
this.appendDeferredTriggers(block, block.triggers);
21492038
this.appendDeferredTriggers(block, block.prefetchTriggers);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,5 +800,4 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
800800
missingRequiredInputs(): void {}
801801
illegalForLoopTrackAccess(): void {}
802802
inaccessibleDeferredTriggerElement(): void {}
803-
controlFlowPreventingContentProjection(): void {}
804803
}

0 commit comments

Comments
 (0)