angular · crisbeto · Nov 8, 2023 · Nov 8, 2023 · Nov 8, 2023 · Nov 27, 2023
diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md
@@ -25,6 +25,7 @@ export enum ErrorCode {
    // (undocumented)
    CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
    CONFLICTING_INPUT_TRANSFORM = 2020,
+    CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 8011,
    // (undocumented)
    DECORATOR_ARG_NOT_LITERAL = 1001,
    // (undocumented)

diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts
@@ -463,10 +463,7 @@ export class ComponentDecoratorHandler implements
        rawHostDirectives,
        meta: {
          ...metadata,
-          template: {
-            nodes: template.nodes,
-            ngContentSelectors: template.ngContentSelectors,
-          },
+          template,
          encapsulation,
          changeDetection,
          interpolation: template.interpolationConfig ?? DEFAULT_INTERPOLATION_CONFIG,
@@ -546,6 +543,8 @@ export class ComponentDecoratorHandler implements
      schemas: analysis.schemas,
      decorator: analysis.decorator,
      assumedToExportProviders: false,
+      ngContentSelectors: analysis.template.ngContentSelectors,
+      preserveWhitespaces: analysis.template.preserveWhitespaces ?? false,
    });

    this.resourceRegistry.registerResources(analysis.resources, node);
@@ -614,7 +613,7 @@ export class ComponentDecoratorHandler implements
    ctx.addTemplate(
        new Reference(node), binder, meta.template.diagNodes, scope.pipes, scope.schemas,
        meta.template.sourceMapping, meta.template.file, meta.template.errors,
-        meta.meta.isStandalone);
+        meta.meta.isStandalone, meta.meta.template.preserveWhitespaces ?? false);
  }

  extendedTemplateCheck(

diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts
@@ -169,7 +169,9 @@ export class DirectiveDecoratorHandler implements
      isSignal: analysis.meta.isSignal,
      imports: null,
      schemas: null,
+      ngContentSelectors: null,
      decorator: analysis.decorator,
+      preserveWhitespaces: false,
      // Directives analyzed within our own compilation are not _assumed_ to export providers.
      // Instead, we statically analyze their imports to make a direct determination.
      assumedToExportProviders: false,

diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts
@@ -110,6 +110,8 @@ runInEachFileSystem(() => {
        selector: '[dir]',
        isStructural: false,
        animationTriggerNames: null,
+        ngContentSelectors: null,
+        preserveWhitespaces: false,
      };
      matcher.addSelectables(CssSelector.parse('[dir]'), [dirMeta]);


diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
@@ -286,6 +286,21 @@ export enum ErrorCode {
   */
  INACCESSIBLE_DEFERRED_TRIGGER_ELEMENT = 8010,

+  /**
+   * A control flow node is projected at the root of a component and is preventing its direct
+   * descendants from being projected, because it has more than one root node.
+   *
+   * ```
+   * <comp>
+   *  @if (expr) {
+   *    <div projectsIntoSlot></div>
+   *    Text preventing the div from being projected
+   *  }
+   * </comp>
+   * ```
+   */
+  CONTROL_FLOW_PREVENTING_CONTENT_PROJECTION = 8011,
+
  /**
   * A two way binding in a template has an incorrect syntax,
   * parentheses outside brackets. For example:

diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/util.ts b/packages/compiler-cli/src/ngtsc/indexer/test/util.ts
@@ -56,6 +56,8 @@ export function getBoundTemplate(
                             exportAs: null,
                             isStructural: false,
                             animationTriggerNames: null,
+                             ngContentSelectors: null,
+                             preserveWhitespaces: false,
                           }]);
  });
  const binder = new R3TargetBinder(matcher);

diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
@@ -96,6 +96,9 @@ export class DtsMetadataReader implements MetadataReader {
          param.typeValueReference.importedName === 'TemplateRef';
    });

+    const ngContentSelectors =
+        def.type.typeArguments.length > 6 ? readStringArrayType(def.type.typeArguments[6]) : null;
+
    const isStandalone =
        def.type.typeArguments.length > 7 && (readBooleanType(def.type.typeArguments[7]) ?? false);

@@ -126,6 +129,7 @@ export class DtsMetadataReader implements MetadataReader {
      isPoisoned: false,
      isStructural,
      animationTriggerNames: null,
+      ngContentSelectors,
      isStandalone,
      isSignal,
      // Imports are tracked in metadata only for template type-checking purposes,
@@ -136,6 +140,9 @@ export class DtsMetadataReader implements MetadataReader {
      decorator: null,
      // Assume that standalone components from .d.ts files may export providers.
      assumedToExportProviders: isComponent && isStandalone,
+      // `preserveWhitespaces` isn't encoded in the .d.ts and is only
+      // used to increase the accuracy of a diagnostic.
+      preserveWhitespaces: false,
    };
  }


diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
@@ -330,6 +330,8 @@ function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {
    decorator: null,
    hostDirectives: null,
    assumedToExportProviders: false,
+    ngContentSelectors: null,
+    preserveWhitespaces: false,
  };
 }


diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts
@@ -84,6 +84,11 @@ export interface TypeCheckBlockMetadata {
   * A boolean indicating whether the component is standalone.
   */
  isStandalone: boolean;
+
+  /**
+   * A boolean indicating whether the component preserves whitespaces in its template.
+   */
+  preserveWhitespaces: boolean;
 }

 export interface TypeCtorMetadata {

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/context.ts
@@ -37,13 +37,15 @@ export interface TypeCheckContext {
   * @param file the `ParseSourceFile` associated with the template.
   * @param parseErrors the `ParseError`'s associated with the template.
   * @param isStandalone a boolean indicating whether the component is standalone.
+   * @param preserveWhitespaces a boolean indicating whether the component's template preserves
+   * whitespaces.
   */
  addTemplate(
      ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
      binder: R3TargetBinder<TypeCheckableDirectiveMeta>, template: TmplAstNode[],
      pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>,
      schemas: SchemaMetadata[], sourceMapping: TemplateSourceMapping, file: ParseSourceFile,
-      parseErrors: ParseError[]|null, isStandalone: boolean): void;
+      parseErrors: ParseError[]|null, isStandalone: boolean, preserveWhitespaces: boolean): void;
 }

 /**

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
@@ -210,7 +210,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
      binder: R3TargetBinder<TypeCheckableDirectiveMeta>, template: TmplAstNode[],
      pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>,
      schemas: SchemaMetadata[], sourceMapping: TemplateSourceMapping, file: ParseSourceFile,
-      parseErrors: ParseError[]|null, isStandalone: boolean): void {
+      parseErrors: ParseError[]|null, isStandalone: boolean, preserveWhitespaces: boolean): void {
    if (!this.host.shouldCheckComponent(ref.node)) {
      return;
    }
@@ -293,7 +293,8 @@ export class TypeCheckContextImpl implements TypeCheckContext {
      boundTarget,
      pipes,
      schemas,
-      isStandalone
+      isStandalone,
+      preserveWhitespaces,
    };
    this.perf.eventCount(PerfEvent.GenerateTcb);
    if (inliningRequirement !== TcbInliningRequirement.None &&

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
@@ -6,7 +6,7 @@
 * found in the LICENSE file at https://angular.io/license
 */

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

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

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

 function makeInlineDiagnostic(

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -6,7 +6,7 @@
 * found in the LICENSE file at https://angular.io/license
 */

-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';
+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';
 import ts from 'typescript';

 import {Reference} from '../../imports';
@@ -84,7 +84,7 @@ export function generateTypeCheckBlock(
    genericContextBehavior: TcbGenericContextBehavior): ts.FunctionDeclaration {
  const tcb = new Context(
      env, domSchemaChecker, oobRecorder, meta.id, meta.boundTarget, meta.pipes, meta.schemas,
-      meta.isStandalone);
+      meta.isStandalone, meta.preserveWhitespaces);
  const scope = Scope.forNodes(tcb, null, null, tcb.boundTarget.target.template!, /* guard */ null);
  const ctxRawType = env.referenceType(ref);
  if (!ts.isTypeReferenceNode(ctxRawType)) {
@@ -905,6 +905,100 @@ class TcbDomSchemaCheckerOp extends TcbOp {
 }


+/**
+ * A `TcbOp` that finds and flags control flow nodes that interfere with content projection.
+ *
+ * Context:
+ * `@if` and `@for` try to emulate the content projection behavior of `*ngIf` and `*ngFor`
+ * in order to reduce breakages when moving from one syntax to the other (see #52414), however the
+ * approach only works if there's only one element at the root of the control flow expression.
+ * This means that a stray sibling node (e.g. text) can prevent an element from being projected
+ * into the right slot. The purpose of the `TcbOp` is to find any places where a node at the root
+ * of a control flow expression *would have been projected* into a specific slot, if the control
+ * flow node didn't exist.
+ */
+class TcbControlFlowContentProjectionOp extends TcbOp {
+  constructor(
+      private tcb: Context, private element: TmplAstElement, private ngContentSelectors: string[],
+      private componentName: string) {
+    super();
+  }
+
+  override readonly optional = false;
+
+  override execute(): null {
+    const controlFlowToCheck = this.findPotentialControlFlowNodes();
+
+    if (controlFlowToCheck.length > 0) {
+      const matcher = new SelectorMatcher<string>();
+
+      for (const selector of this.ngContentSelectors) {
+        // `*` is a special selector for the catch-all slot.
+        if (selector !== '*') {
+          matcher.addSelectables(CssSelector.parse(selector), selector);
+        }
+      }
+
+      for (const root of controlFlowToCheck) {
+        for (const child of root.children) {
+          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.hostPreserveWhitespaces);
+            });
+          }
+        }
+      }
+    }
+
+    return null;
+  }
+
+  private findPotentialControlFlowNodes() {
+    const result: Array<TmplAstIfBlockBranch|TmplAstForLoopBlock> = [];
+
+    for (const child of this.element.children) {
+      let eligibleNode: TmplAstForLoopBlock|TmplAstIfBlockBranch|null = null;
+
+      // Only `@for` blocks and the first branch of `@if` blocks participate in content projection.
+      if (child instanceof TmplAstForLoopBlock) {
+        eligibleNode = child;
+      } else if (child instanceof TmplAstIfBlock) {
+        eligibleNode = child.branches[0];  // @if blocks are guaranteed to have at least one branch.
+      }
+
+      // Skip nodes with less than two children since it's impossible
+      // for them to run into the issue that we're checking for.
+      if (eligibleNode === null || eligibleNode.children.length < 2) {
+        continue;
+      }
+
+      // Count the number of root nodes while skipping empty text where relevant.
+      const rootNodeCount = eligibleNode.children.reduce((count, node) => {
+        // Normally `preserveWhitspaces` would have been accounted for during parsing, however
+        // in `ngtsc/annotations/component/src/resources.ts#parseExtractedTemplate` we enable
+        // `preserveWhitespaces` to preserve the accuracy of source maps diagnostics. This means
+        // that we have to account for it here since the presence of text nodes affects the
+        // content projection behavior.
+        if (!(node instanceof TmplAstText) || this.tcb.hostPreserveWhitespaces ||
+            node.value.trim().length > 0) {
+          count++;
+        }
+
+        return count;
+      }, 0);
+
+      // Content projection can only be affected if there is more than one root node.
+      if (rootNodeCount > 1) {
+        result.push(eligibleNode);
+      }
+    }
+
+    return result;
+  }
+}
+
 /**
 * Mapping between attributes names that don't correspond to their element property names.
 * Note: this mapping has to be kept in sync with the equally named mapping in the runtime.
@@ -1488,7 +1582,8 @@ export class Context {
      readonly oobRecorder: OutOfBandDiagnosticRecorder, readonly id: TemplateId,
      readonly boundTarget: BoundTarget<TypeCheckableDirectiveMeta>,
      private pipes: Map<string, Reference<ClassDeclaration<ts.ClassDeclaration>>>,
-      readonly schemas: SchemaMetadata[], readonly hostIsStandalone: boolean) {}
+      readonly schemas: SchemaMetadata[], readonly hostIsStandalone: boolean,
+      readonly hostPreserveWhitespaces: boolean) {}

  /**
   * Allocate a new variable name for use within the `Context`.
@@ -1837,6 +1932,7 @@ 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);
      this.appendDirectivesAndInputsOfNode(node);
      this.appendOutputsOfNode(node);
      this.appendChildren(node);
@@ -2033,6 +2129,22 @@ class Scope {
    }
  }

+  private appendContentProjectionCheckOp(root: TmplAstElement): void {
+    const meta =
+        this.tcb.boundTarget.getDirectivesOfNode(root)?.find(meta => meta.isComponent) || null;
+
+    if (meta !== null && meta.ngContentSelectors !== null && meta.ngContentSelectors.length > 0) {
+      const selectors = meta.ngContentSelectors;
+
+      // We don't need to generate anything for components that don't have projection
+      // slots, or they only have one catch-all slot (represented by `*`).
+      if (selectors.length > 1 || (selectors.length === 1 && selectors[0] !== '*')) {
+        this.opQueue.push(
+            new TcbControlFlowContentProjectionOp(this.tcb, root, selectors, meta.name));
+      }
+    }
+  }
+
  private appendDeferredBlock(block: TmplAstDeferredBlock): void {
    this.appendDeferredTriggers(block, block.triggers);
    this.appendDeferredTriggers(block, block.prefetchTriggers);