From 37483926eaaa508cc606a8ddf8e1bcae3a2dd70c Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 7 Oct 2024 11:37:10 +0000 Subject: [PATCH] refactor(migrations): speed up reference resolution in templates (#58106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of inspecting all types of property accesses, we can use a similar optimization we did for TS references— leveraging the list of known field names to minimize type checker calls. PR Close #58106 --- .../identify_host_references.ts | 2 ++ .../identify_template_references.ts | 15 +++++++++++---- .../src/passes/reference_resolution/index.ts | 11 ++++++++++- .../template_reference_visitor.ts | 10 ++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_host_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_host_references.ts index 8486425fc0b..6fb02456ac7 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_host_references.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_host_references.ts @@ -38,6 +38,7 @@ export function identifyHostBindingReferences( reflector: ReflectionHost, result: ReferenceResult, knownFields: KnownFields, + fieldNamesToConsiderForReferenceLookup: Set | null, ) { if (node.name === undefined) { return; @@ -97,6 +98,7 @@ export function identifyHostBindingReferences( null, node, knownFields, + fieldNamesToConsiderForReferenceLookup, ); for (const [rawName, expression] of hostMap.entries()) { diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_template_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_template_references.ts index d593efcd66b..f2711cce477 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_template_references.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/identify_template_references.ts @@ -14,7 +14,7 @@ import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; import {PartialEvaluator} from '@angular/compiler-cli/src/ngtsc/partial_evaluator'; import {ClassDeclaration, ReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection'; import {CompilationMode} from '@angular/compiler-cli/src/ngtsc/transform'; -import {TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; +import {OptimizeFor, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import ts from 'typescript'; import {ProgramInfo, projectFile} from '../../../../../utils/tsurge'; import {TemplateReferenceVisitor} from './template_reference_visitor'; @@ -38,9 +38,10 @@ export function identifyTemplateReferences( options: NgCompilerOptions, result: ReferenceResult, knownFields: KnownFields, + fieldNamesToConsiderForReferenceLookup: Set | null, ) { const template = - templateTypeChecker.getTemplate(node) ?? + templateTypeChecker.getTemplate(node, OptimizeFor.WholeProgram) ?? // If there is no template registered in the TCB or compiler, the template may // be skipped due to an explicit `jit: true` setting. We try to detect this case // and parse the template manually. @@ -54,7 +55,13 @@ export function identifyTemplateReferences( ); if (template !== null) { - const visitor = new TemplateReferenceVisitor(checker, templateTypeChecker, node, knownFields); + const visitor = new TemplateReferenceVisitor( + checker, + templateTypeChecker, + node, + knownFields, + fieldNamesToConsiderForReferenceLookup, + ); template.forEach((node) => node.visit(visitor)); for (const res of visitor.result) { @@ -67,7 +74,7 @@ export function identifyTemplateReferences( if (templateFilePath === '') { // TODO: Incorporate a TODO potentially. console.error( - `Found reference to input ${res.targetField.key} that cannot be ` + + `Found reference to field ${res.targetField.key} that cannot be ` + `migrated because the template cannot be parsed with source map information ` + `(in file: ${node.getSourceFile().fileName}).`, ); diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/index.ts b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/index.ts index d1dd5a3d606..114a1b2bcc6 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/index.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/index.ts @@ -79,11 +79,20 @@ export function createFindAllSourceFileReferencesVisitor, + fieldNamesToConsiderForReferenceLookup: Set | null, ) { super(); this.expressionVisitor = new TemplateExpressionReferenceVisitor( @@ -88,6 +89,7 @@ export class TemplateReferenceVisitor< templateTypeChecker, componentClass, knownFields, + fieldNamesToConsiderForReferenceLookup, ); } @@ -244,6 +246,7 @@ export class TemplateExpressionReferenceVisitor< private templateTypeChecker: TemplateTypeChecker | null, private componentClass: ts.ClassDeclaration, private knownFields: KnownFields, + private fieldNamesToConsiderForReferenceLookup: Set | null, ) { super(); } @@ -294,6 +297,13 @@ export class TemplateExpressionReferenceVisitor< * a known field. If so, the result is captured. */ private _inspectPropertyAccess(ast: PropertyRead | PropertyWrite, astPath: AST[]) { + if ( + this.fieldNamesToConsiderForReferenceLookup !== null && + !this.fieldNamesToConsiderForReferenceLookup.has(ast.name) + ) { + return; + } + const isWrite = !!( ast instanceof PropertyWrite || (this.activeTmplAstNode && isTwoWayBindingNode(this.activeTmplAstNode))