diff --git a/packages/core/schematics/migrations/signal-migration/src/input_detection/template_reference_visitor.ts b/packages/core/schematics/migrations/signal-migration/src/input_detection/template_reference_visitor.ts index 80161bb5b9e..7e38d1a334c 100644 --- a/packages/core/schematics/migrations/signal-migration/src/input_detection/template_reference_visitor.ts +++ b/packages/core/schematics/migrations/signal-migration/src/input_detection/template_reference_visitor.ts @@ -38,7 +38,7 @@ import { import {KnownInputs} from './known_inputs'; import {attemptRetrieveInputFromSymbol} from './nodes_to_input'; import {MigrationHost} from '../migration_host'; -import {InputDescriptor} from '../utils/input_id'; +import {InputDescriptor, InputUniqueKey} from '../utils/input_id'; import {InputIncompatibilityReason} from './incompatibility'; import {BoundAttribute, BoundEvent} from '../../../../../../compiler/src/render3/r3_ast'; @@ -51,8 +51,8 @@ export interface TmplInputExpressionReference { targetInput: InputDescriptor; read: PropertyRead; context: ExprContext; - isInsideNarrowingExpression: boolean; isObjectShorthandExpression: boolean; + isLikelyNarrowed: boolean; } /** @@ -67,12 +67,15 @@ export class TemplateReferenceVisitor extends TmplAstRecursiveVisitor { /** * Whether we are currently descending into HTML AST nodes - * where bound attributes should be considered "narrowed". + * where all bound attributes are considered potentially narrowing. * - * TODO: Remove with: https://github.com/angular/angular/pull/55456. + * Keeps track of all referenced inputs in such attribute expressions. */ - private isAttributeInsideNarrowingExpression = false; + private templateAttributeReferencedInputs: TmplInputExpressionReference[] | null = + null; + private expressionVisitor: TemplateExpressionReferenceVisitor; + private seenInputsCount = new Map(); constructor( host: MigrationHost, @@ -88,16 +91,56 @@ export class TemplateReferenceVisitor extends TmplAstRecursiveVisitor { templateTypeChecker, componentClass, knownInputs, - this.result, ); } + private checkExpressionForReferencedInputs(activeNode: TmplAstNode, expressionNode: AST) { + const referencedInputs = this.expressionVisitor.checkTemplateExpression( + activeNode, + expressionNode, + ); + // Add all references to the overall visitor result. + this.result.push(...referencedInputs); + + // Count usages of seen input references. We'll use this to make decisions + // based on whether inputs are potentially narrowed or not. + for (const input of referencedInputs) { + this.seenInputsCount.set( + input.targetInput.key, + (this.seenInputsCount.get(input.targetInput.key) ?? 0) + 1, + ); + } + + return referencedInputs; + } + + private descendAndCheckForNarrowedSimilarReferences( + potentiallyNarrowedInputs: TmplInputExpressionReference[], + descend: () => void, + ) { + const inputs = potentiallyNarrowedInputs.map((i) => ({ + ref: i, + key: i.targetInput.key, + pastCount: this.seenInputsCount.get(i.targetInput.key) ?? 0, + })); + + descend(); + + for (const input of inputs) { + // Input was referenced inside a narrowable spot, and is used in child nodes. + // This is a sign for the input to be narrowed. Mark it as such. + if ((this.seenInputsCount.get(input.key) ?? 0) > input.pastCount) { + input.ref.isLikelyNarrowed = true; + } + } + } + override visitTemplate(template: TmplAstTemplate): void { // Note: We assume all bound expressions for templates may be subject // to TCB narrowing. This is relevant for now until we support narrowing // of signal calls in templates. // TODO: Remove with: https://github.com/angular/angular/pull/55456. - this.isAttributeInsideNarrowingExpression = true; + this.templateAttributeReferencedInputs = []; tmplAstVisitAll(this, template.attributes); tmplAstVisitAll(this, template.templateAttrs); @@ -110,98 +153,77 @@ export class TemplateReferenceVisitor extends TmplAstRecursiveVisitor { tmplAstVisitAll(this, template.outputs); } - this.isAttributeInsideNarrowingExpression = false; + const referencedInputs = this.templateAttributeReferencedInputs; + this.templateAttributeReferencedInputs = null; - tmplAstVisitAll(this, template.children); - tmplAstVisitAll(this, template.references); - tmplAstVisitAll(this, template.variables); + this.descendAndCheckForNarrowedSimilarReferences(referencedInputs, () => { + tmplAstVisitAll(this, template.children); + tmplAstVisitAll(this, template.references); + tmplAstVisitAll(this, template.variables); + }); } override visitIfBlockBranch(block: TmplAstIfBlockBranch): void { if (block.expression) { - this.expressionVisitor.checkTemplateExpression( - block, - /* isInsideNarrowingExpression */ true, - block.expression, - ); + const referencedInputs = this.checkExpressionForReferencedInputs(block, block.expression); + this.descendAndCheckForNarrowedSimilarReferences(referencedInputs, () => { + super.visitIfBlockBranch(block); + }); + } else { + super.visitIfBlockBranch(block); } - super.visitIfBlockBranch(block); } override visitForLoopBlock(block: TmplAstForLoopBlock): void { - this.expressionVisitor.checkTemplateExpression( - block, - /* isInsideNarrowingExpression */ false, - block.expression, - ); - this.expressionVisitor.checkTemplateExpression( - block, - /* isInsideNarrowingExpression */ false, - block.trackBy, - ); + this.checkExpressionForReferencedInputs(block, block.expression); + this.checkExpressionForReferencedInputs(block, block.trackBy); super.visitForLoopBlock(block); } override visitSwitchBlock(block: TmplAstSwitchBlock): void { - this.expressionVisitor.checkTemplateExpression( - block, - /* isInsideNarrowingExpression */ true, - block.expression, - ); - super.visitSwitchBlock(block); + const referencedInputs = this.checkExpressionForReferencedInputs(block, block.expression); + this.descendAndCheckForNarrowedSimilarReferences(referencedInputs, () => { + super.visitSwitchBlock(block); + }); } override visitSwitchBlockCase(block: TmplAstSwitchBlockCase): void { if (block.expression) { - this.expressionVisitor.checkTemplateExpression( - block, - /* isInsideNarrowingExpression */ true, - block.expression, - ); + const referencedInputs = this.checkExpressionForReferencedInputs(block, block.expression); + this.descendAndCheckForNarrowedSimilarReferences(referencedInputs, () => { + super.visitSwitchBlockCase(block); + }); + } else { + super.visitSwitchBlockCase(block); } - super.visitSwitchBlockCase(block); } override visitDeferredBlock(deferred: TmplAstDeferredBlock): void { if (deferred.triggers.when) { - this.expressionVisitor.checkTemplateExpression( - deferred, - /* isInsideNarrowingExpression */ false, - deferred.triggers.when.value, - ); + this.checkExpressionForReferencedInputs(deferred, deferred.triggers.when.value); } if (deferred.prefetchTriggers.when) { - this.expressionVisitor.checkTemplateExpression( - deferred, - /* isInsideNarrowingExpression */ false, - deferred.prefetchTriggers.when.value, - ); + this.checkExpressionForReferencedInputs(deferred, deferred.prefetchTriggers.when.value); } super.visitDeferredBlock(deferred); } override visitBoundText(text: TmplAstBoundText): void { - this.expressionVisitor.checkTemplateExpression( - text, - /* isInsideNarrowingExpression */ false, - text.value, - ); + this.checkExpressionForReferencedInputs(text, text.value); } override visitBoundEvent(attribute: TmplAstBoundEvent): void { - this.expressionVisitor.checkTemplateExpression( - attribute, - /* isInsideNarrowingExpression */ false, - attribute.handler, - ); + this.checkExpressionForReferencedInputs(attribute, attribute.handler); } override visitBoundAttribute(attribute: TmplAstBoundAttribute): void { - this.expressionVisitor.checkTemplateExpression( - attribute, - /* isInsideNarrowingExpression */ this.isAttributeInsideNarrowingExpression, - attribute.value, - ); + const referencedInputs = this.checkExpressionForReferencedInputs(attribute, attribute.value); + + // Attributes inside templates are potentially "narrowed" and hence we + // keep track of all referenced inputs to see if they actually are. + if (this.templateAttributeReferencedInputs !== null) { + this.templateAttributeReferencedInputs.push(...referencedInputs); + } } } @@ -214,7 +236,8 @@ export class TemplateReferenceVisitor extends TmplAstRecursiveVisitor { */ export class TemplateExpressionReferenceVisitor extends RecursiveAstVisitor { private activeTmplAstNode: ExprContext | null = null; - private isInsideNarrowingExpression = false; + private detectedInputReferences: TmplInputExpressionReference[] = []; + private isInsideObjectShorthandExpression = false; constructor( @@ -223,7 +246,6 @@ export class TemplateExpressionReferenceVisitor extends RecursiveAs private templateTypeChecker: TemplateTypeChecker | null, private componentClass: ts.ClassDeclaration, private knownInputs: KnownInputs, - private result: TmplInputExpressionReference[], ) { super(); } @@ -231,13 +253,13 @@ export class TemplateExpressionReferenceVisitor extends RecursiveAs /** Checks the given AST expression. */ checkTemplateExpression( activeNode: ExprContext, - isInsideNarrowingExpression: boolean, expressionNode: AST, - ) { + ): TmplInputExpressionReference[] { + this.detectedInputReferences = []; this.activeTmplAstNode = activeNode; - this.isInsideNarrowingExpression = isInsideNarrowingExpression; expressionNode.visit(this); + return this.detectedInputReferences; } // Keep track when we are inside an object shorthand expression. This is @@ -316,12 +338,12 @@ export class TemplateExpressionReferenceVisitor extends RecursiveAs return null; } - this.result.push({ + this.detectedInputReferences.push({ target: targetInput.descriptor.node, targetInput: targetInput.descriptor, read: ast, context: this.activeTmplAstNode!, - isInsideNarrowingExpression: this.isInsideNarrowingExpression, + isLikelyNarrowed: false, isObjectShorthandExpression: this.isInsideObjectShorthandExpression, }); @@ -365,12 +387,12 @@ export class TemplateExpressionReferenceVisitor extends RecursiveAs return null; } - this.result.push({ + this.detectedInputReferences.push({ target: matchingTarget.descriptor.node, targetInput: matchingTarget.descriptor, read: ast, context: this.activeTmplAstNode!, - isInsideNarrowingExpression: this.isInsideNarrowingExpression, + isLikelyNarrowed: false, isObjectShorthandExpression: this.isInsideObjectShorthandExpression, }); return matchingTarget.descriptor; diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_host_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_host_references.ts index 955814afd77..f7e9a38838d 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_host_references.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_host_references.ts @@ -95,13 +95,12 @@ export function identifyHostBindingReferences( } const hostMap = reflectObjectLiteral(hostField); const expressionResult: TmplInputExpressionReference[] = []; - const expressionVisitor = new TemplateExpressionReferenceVisitor( + const expressionVisitor = new TemplateExpressionReferenceVisitor( host, checker, null, node, knownDecoratorInputs, - expressionResult, ); for (const [rawName, expression] of hostMap.entries()) { @@ -158,7 +157,7 @@ export function identifyHostBindingReferences( } if (parsed != null) { - expressionVisitor.checkTemplateExpression(expression, false, parsed); + expressionResult.push(...expressionVisitor.checkTemplateExpression(expression, parsed)); } } diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_template_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_template_references.ts index 7bdfbb4d453..e9a50c44348 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_template_references.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/references/identify_template_references.ts @@ -55,10 +55,7 @@ export function identifyTemplateReferences( // TODO: Remove this when we support signal narrowing in templates. // https://github.com/angular/angular/pull/55456. - if ( - process.env['MIGRATE_NARROWED_NARROWED_IN_TEMPLATES'] !== '1' && - res.isInsideNarrowingExpression - ) { + if (process.env['MIGRATE_NARROWED_NARROWED_IN_TEMPLATES'] !== '1' && res.isLikelyNarrowed) { knownInputs.markInputAsIncompatible(res.targetInput, { reason: InputIncompatibilityReason.NarrowedInTemplateButNotSupportedYetTODO, context: null, diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/template_ng_if.ts b/packages/core/schematics/migrations/signal-migration/test/golden-test/template_ng_if.ts index a6c6e733a0b..366458f0bf8 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden-test/template_ng_if.ts +++ b/packages/core/schematics/migrations/signal-migration/test/golden-test/template_ng_if.ts @@ -15,10 +15,20 @@ import {Component, Input} from '@angular/core';
{{third}}
+ +
+ {{notTheInput}} +
+ + @if (fifth) { + {{notTheInput}} + } `, }) export class MyComp { @Input() first = true; @Input() second = false; @Input() third = true; + @Input() fourth = true; + @Input() fifth = true; } diff --git a/packages/core/schematics/migrations/signal-migration/test/golden.txt b/packages/core/schematics/migrations/signal-migration/test/golden.txt index 48b08927005..abc4bc09b82 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden.txt +++ b/packages/core/schematics/migrations/signal-migration/test/golden.txt @@ -848,7 +848,7 @@ spyOn(new MyComp(), 'myInput').and.returnValue(); // tslint:disable -import {Component, Input} from '@angular/core'; +import {Component, Input, input} from '@angular/core'; @Component({ template: ` @@ -863,12 +863,22 @@ import {Component, Input} from '@angular/core';
{{third}}
+ +
+ {{notTheInput}} +
+ + @if (fifth()) { + {{notTheInput}} + } `, }) export class MyComp { @Input() first = true; @Input() second = false; @Input() third = true; + fourth = input(true); + fifth = input(true); } @@@@@@ template_object_shorthand.ts @@@@@@