From d608b857c689d17a7ffa33bbb510301014d24a17 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 12 Aug 2024 15:32:37 +0000 Subject: [PATCH] refactor(migrations): handle `jit: true` component templates in signal input migration (#57347) Components with `jit: true` are not processed by the Angular compiler, so we cannot ask the template checker for the parsed template; simply because the template wasn't attempted to be parsed. We still can migrate simple cases of such components, commonly seen in unit tests. We do this by manually parsing the template and making use of the reference fallback resolution that is also used for host bindings (where we don't have any type check block information). PR Close #57347 --- .../signal-migration/src/BUILD.bazel | 1 + .../signal-migration/src/create_program.ts | 14 +-- .../passes/2_find_source_file_references.ts | 17 +++- .../identify_template_references.ts | 77 +++++++++++++++- .../signal-migration/src/phase_analysis.ts | 3 + .../src/utils/extract_template.ts | 90 +++++++++++++++++++ .../jit_true_component_external_tmpl.html | 1 + .../test/golden-test/jit_true_components.ts | 27 ++++++ .../signal-migration/test/golden.txt | 32 +++++++ .../test/golden_best_effort.txt | 32 +++++++ 10 files changed, 285 insertions(+), 9 deletions(-) create mode 100644 packages/core/schematics/migrations/signal-migration/src/utils/extract_template.ts create mode 100644 packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_component_external_tmpl.html create mode 100644 packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_components.ts diff --git a/packages/core/schematics/migrations/signal-migration/src/BUILD.bazel b/packages/core/schematics/migrations/signal-migration/src/BUILD.bazel index 743ecfe1b6f..c3a13e77c70 100644 --- a/packages/core/schematics/migrations/signal-migration/src/BUILD.bazel +++ b/packages/core/schematics/migrations/signal-migration/src/BUILD.bazel @@ -15,6 +15,7 @@ ts_library( "//packages/compiler-cli", "//packages/compiler-cli/src/ngtsc/annotations", "//packages/compiler-cli/src/ngtsc/annotations/common", + "//packages/compiler-cli/src/ngtsc/annotations/component", "//packages/compiler-cli/src/ngtsc/annotations/directive", "//packages/compiler-cli/src/ngtsc/core", "//packages/compiler-cli/src/ngtsc/core:api", diff --git a/packages/core/schematics/migrations/signal-migration/src/create_program.ts b/packages/core/schematics/migrations/signal-migration/src/create_program.ts index 314a8eb6bee..ef2b1ca85f7 100644 --- a/packages/core/schematics/migrations/signal-migration/src/create_program.ts +++ b/packages/core/schematics/migrations/signal-migration/src/create_program.ts @@ -15,15 +15,16 @@ import {PartialEvaluator} from '../../../../../compiler-cli/src/ngtsc/partial_ev import {NgtscProgram} from '../../../../../compiler-cli/src/ngtsc/program'; import {TypeScriptReflectionHost} from '../../../../../compiler-cli/src/ngtsc/reflection'; +import assert from 'assert'; +import {ResourceLoader} from '../../../../../compiler-cli/src/ngtsc/annotations'; +import {NgCompiler} from '../../../../../compiler-cli/src/ngtsc/core'; +import {ReferenceEmitter} from '../../../../../compiler-cli/src/ngtsc/imports'; +import {isShim} from '../../../../../compiler-cli/src/ngtsc/shims'; +import {TemplateTypeChecker} from '../../../../../compiler-cli/src/ngtsc/typecheck/api'; import { ParsedConfiguration, readConfiguration, } from '../../../../../compiler-cli/src/perform_compile'; -import {TemplateTypeChecker} from '../../../../../compiler-cli/src/ngtsc/typecheck/api'; -import {ReferenceEmitter} from '../../../../../compiler-cli/src/ngtsc/imports'; -import {isShim} from '../../../../../compiler-cli/src/ngtsc/shims'; -import {NgCompiler} from '../../../../../compiler-cli/src/ngtsc/core'; -import assert from 'assert'; /** * Interface containing the analysis information @@ -41,6 +42,7 @@ export interface AnalysisProgramInfo { dtsMetadataReader: DtsMetadataReader; evaluator: PartialEvaluator; refEmitter: ReferenceEmitter; + resourceLoader: ResourceLoader; } /** Creates and prepares analysis for the given TypeScript project. */ @@ -116,6 +118,7 @@ export function prepareAnalysisInfo( const reflector = new TypeScriptReflectionHost(typeChecker); const evaluator = new PartialEvaluator(reflector, typeChecker, null); const dtsMetadataReader = new DtsMetadataReader(typeChecker, reflector); + const resourceLoader = compiler['resourceManager']; const limitToRootNamesOnly = process.env['LIMIT_TO_ROOT_NAMES_ONLY'] === '1'; if (limitToRootNamesOnly) { @@ -148,5 +151,6 @@ export function prepareAnalysisInfo( typeChecker, refEmitter, templateTypeChecker, + resourceLoader, }; } diff --git a/packages/core/schematics/migrations/signal-migration/src/passes/2_find_source_file_references.ts b/packages/core/schematics/migrations/signal-migration/src/passes/2_find_source_file_references.ts index d3eff5d4a1e..9c40ce47d4b 100644 --- a/packages/core/schematics/migrations/signal-migration/src/passes/2_find_source_file_references.ts +++ b/packages/core/schematics/migrations/signal-migration/src/passes/2_find_source_file_references.ts @@ -20,6 +20,8 @@ import {identifyPotentialTypeScriptReference} from './references/identify_ts_ref import {PartialDirectiveTypeInCatalystTests} from '../pattern_advisors/partial_directive_type'; import {InputReferenceKind} from '../utils/input_reference'; import {GroupedTsAstVisitor} from '../utils/grouped_ts_ast_visitor'; +import {ResourceLoader} from '../../../../../../compiler-cli/src/ngtsc/annotations'; +import {PartialEvaluator} from '../../../../../../compiler-cli/src/ngtsc/partial_evaluator'; /** * Phase where we iterate through all source file references and @@ -38,6 +40,8 @@ export function pass2_IdentifySourceFileReferences( host: MigrationHost, checker: ts.TypeChecker, reflector: ReflectionHost, + resourceLoader: ResourceLoader, + evaluator: PartialEvaluator, templateTypeChecker: TemplateTypeChecker, groupedTsAstVisitor: GroupedTsAstVisitor, knownInputs: KnownInputs, @@ -51,7 +55,18 @@ export function pass2_IdentifySourceFileReferences( const visitor = (node: ts.Node) => { if (ts.isClassDeclaration(node)) { - identifyTemplateReferences(node, host, checker, templateTypeChecker, result, knownInputs); + identifyTemplateReferences( + node, + host, + reflector, + checker, + evaluator, + templateTypeChecker, + resourceLoader, + host.options, + result, + knownInputs, + ); identifyHostBindingReferences(node, host, checker, reflector, result, knownInputs); } 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 d6a0e48ba6b..e4ba0cb2640 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 @@ -7,13 +7,24 @@ */ import ts from 'typescript'; +import {ResourceLoader} from '../../../../../../../compiler-cli/src/ngtsc/annotations'; +import {TemplateTypeChecker} from '../../../../../../../compiler-cli/src/ngtsc/typecheck/api'; import {InputIncompatibilityReason} from '../../input_detection/incompatibility'; import {KnownInputs} from '../../input_detection/known_inputs'; +import {TemplateReferenceVisitor} from '../../input_detection/template_reference_visitor'; import {MigrationHost} from '../../migration_host'; import {MigrationResult} from '../../result'; import {InputReferenceKind} from '../../utils/input_reference'; -import {TemplateTypeChecker} from '../../../../../../../compiler-cli/src/ngtsc/typecheck/api'; -import {TemplateReferenceVisitor} from '../../input_detection/template_reference_visitor'; +import { + ClassDeclaration, + ReflectionHost, +} from '../../../../../../../compiler-cli/src/ngtsc/reflection'; +import {PartialEvaluator} from '../../../../../../../compiler-cli/src/ngtsc/partial_evaluator'; +import {extractTemplate} from '../../../../../../../compiler-cli/src/ngtsc/annotations/component/src/resources'; +import {attemptExtractTemplateDefinition} from '../../utils/extract_template'; +import {NgCompilerOptions} from '../../../../../../../compiler-cli/src/ngtsc/core/api'; +import {CompilationMode} from '../../../../../../../compiler-cli/src/ngtsc/transform'; +import {TmplAstNode} from '@angular/compiler'; /** * Checks whether the given class has an Angular template, and resolves @@ -22,12 +33,30 @@ import {TemplateReferenceVisitor} from '../../input_detection/template_reference export function identifyTemplateReferences( node: ts.ClassDeclaration, host: MigrationHost, + reflector: ReflectionHost, checker: ts.TypeChecker, + evaluator: PartialEvaluator, templateTypeChecker: TemplateTypeChecker, + resourceLoader: ResourceLoader, + options: NgCompilerOptions, result: MigrationResult, knownInputs: KnownInputs, ) { - const template = templateTypeChecker.getTemplate(node); + const template = + templateTypeChecker.getTemplate(node) ?? + // 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. + extractTemplateWithoutCompilerAnalysis( + node, + checker, + reflector, + resourceLoader, + evaluator, + options, + host, + ); + if (template !== null) { const visitor = new TemplateReferenceVisitor( host, @@ -64,3 +93,45 @@ export function identifyTemplateReferences( } } } + +/** + * Attempts to extract a `@Component` template from the given class, + * without relying on the `NgCompiler` program analysis. + * + * This is useful for JIT components using `jit: true` which were not + * processed by the Angular compiler, but may still have templates that + * contain references to inputs that we can resolve via the fallback + * reference resolutions (that does not use the type check block). + */ +function extractTemplateWithoutCompilerAnalysis( + node: ts.ClassDeclaration, + checker: ts.TypeChecker, + reflector: ReflectionHost, + resourceLoader: ResourceLoader, + evaluator: PartialEvaluator, + options: NgCompilerOptions, + host: MigrationHost, +): TmplAstNode[] | null { + if (node.name === undefined) { + return null; + } + const tmplDef = attemptExtractTemplateDefinition(node, checker, reflector, host); + if (tmplDef === null) { + return null; + } + return extractTemplate( + node as ClassDeclaration, + tmplDef, + evaluator, + null, + resourceLoader, + { + enableBlockSyntax: true, + enableLetSyntax: true, + usePoisonedData: true, + enableI18nLegacyMessageIdFormat: options.enableI18nLegacyMessageIdFormat !== false, + i18nNormalizeLineEndingsInICUs: options.i18nNormalizeLineEndingsInICUs === true, + }, + CompilationMode.FULL, + ).nodes; +} diff --git a/packages/core/schematics/migrations/signal-migration/src/phase_analysis.ts b/packages/core/schematics/migrations/signal-migration/src/phase_analysis.ts index f02204b32d2..ef06d1cf536 100644 --- a/packages/core/schematics/migrations/signal-migration/src/phase_analysis.ts +++ b/packages/core/schematics/migrations/signal-migration/src/phase_analysis.ts @@ -37,6 +37,7 @@ export function executeAnalysisPhase( dtsMetadataReader, typeChecker, templateTypeChecker, + resourceLoader, evaluator, refEmitter, }: AnalysisProgramInfo, @@ -70,6 +71,8 @@ export function executeAnalysisPhase( host, typeChecker, reflector, + resourceLoader, + evaluator, templateTypeChecker, pass2And3SourceFileVisitor, knownInputs, diff --git a/packages/core/schematics/migrations/signal-migration/src/utils/extract_template.ts b/packages/core/schematics/migrations/signal-migration/src/utils/extract_template.ts new file mode 100644 index 00000000000..31cfe77d963 --- /dev/null +++ b/packages/core/schematics/migrations/signal-migration/src/utils/extract_template.ts @@ -0,0 +1,90 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import ts from 'typescript'; +import { + ReflectionHost, + reflectObjectLiteral, +} from '../../../../../../compiler-cli/src/ngtsc/reflection'; +import {MigrationHost} from '../migration_host'; +import {getAngularDecorators} from '../../../../../../compiler-cli/src/ngtsc/annotations'; +import {PartialEvaluator} from './../../../../../../compiler-cli/src/ngtsc/partial_evaluator'; +import { + ExternalTemplateDeclaration, + InlineTemplateDeclaration, +} from '../../../../../../compiler-cli/src/ngtsc/annotations/component/src/resources'; +import {DEFAULT_INTERPOLATION_CONFIG} from '@angular/compiler'; +import path from 'path'; + +/** + * Attempts to extract the `TemplateDefinition` for the given + * class, if possible. + * + * The definition can then be used with the Angular compiler to + * load/parse the given template. + */ +export function attemptExtractTemplateDefinition( + node: ts.ClassDeclaration, + checker: ts.TypeChecker, + reflector: ReflectionHost, + host: MigrationHost, +): InlineTemplateDeclaration | ExternalTemplateDeclaration | null { + const classDecorators = reflector.getDecoratorsOfDeclaration(node); + const evaluator = new PartialEvaluator(reflector, checker, null); + + const ngDecorators = + classDecorators !== null + ? getAngularDecorators(classDecorators, ['Component'], host.isMigratingCore) + : []; + + if ( + ngDecorators.length === 0 || + ngDecorators[0].args === null || + ngDecorators[0].args.length === 0 || + !ts.isObjectLiteralExpression(ngDecorators[0].args[0]) + ) { + return null; + } + + const properties = reflectObjectLiteral(ngDecorators[0].args[0]); + const templateProp = properties.get('template'); + const templateUrlProp = properties.get('templateUrl'); + const containingFile = node.getSourceFile().fileName; + + // inline template. + if (templateProp !== undefined) { + const templateStr = evaluator.evaluate(templateProp); + if (typeof templateStr === 'string') { + return { + isInline: true, + expression: templateProp, + interpolationConfig: DEFAULT_INTERPOLATION_CONFIG, + preserveWhitespaces: false, + resolvedTemplateUrl: containingFile, + templateUrl: containingFile, + } as InlineTemplateDeclaration; + } + } + + // external template. + if (templateUrlProp !== undefined) { + const templateUrl = evaluator.evaluate(templateUrlProp); + if (typeof templateUrl === 'string') { + return { + isInline: false, + interpolationConfig: DEFAULT_INTERPOLATION_CONFIG, + preserveWhitespaces: false, + templateUrlExpression: templateUrlProp, + templateUrl, + resolvedTemplateUrl: path.join(path.dirname(containingFile), templateUrl), + }; + } + } + + return null; +} diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_component_external_tmpl.html b/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_component_external_tmpl.html new file mode 100644 index 00000000000..edc2f0ebff5 --- /dev/null +++ b/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_component_external_tmpl.html @@ -0,0 +1 @@ +{{test}} diff --git a/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_components.ts b/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_components.ts new file mode 100644 index 00000000000..9dae767ed40 --- /dev/null +++ b/packages/core/schematics/migrations/signal-migration/test/golden-test/jit_true_components.ts @@ -0,0 +1,27 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +// tslint:disable + +import {Component, Input} from '@angular/core'; + +@Component({ + jit: true, + template: '{{test}}', +}) +class JitTrueComponent { + @Input() test = true; +} + +@Component({ + jit: true, + templateUrl: './jit_true_component_external_tmpl.html', +}) +class JitTrueComponentExternalTmpl { + @Input() test = true; +} diff --git a/packages/core/schematics/migrations/signal-migration/test/golden.txt b/packages/core/schematics/migrations/signal-migration/test/golden.txt index b99dd3abbc9..381ec49327f 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden.txt +++ b/packages/core/schematics/migrations/signal-migration/test/golden.txt @@ -560,6 +560,38 @@ import { Component, input } from '@angular/core'; export class InlineTmpl { justify = input<'start' | 'end'>('end'); } +@@@@@@ jit_true_component_external_tmpl.html @@@@@@ + +{{test()}} +@@@@@@ jit_true_components.ts @@@@@@ + +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +// tslint:disable + +import { Component, input } from '@angular/core'; + +@Component({ + jit: true, + template: '{{test()}}', +}) +class JitTrueComponent { + test = input(true); +} + +@Component({ + jit: true, + templateUrl: './jit_true_component_external_tmpl.html', +}) +class JitTrueComponentExternalTmpl { + test = input(true); +} @@@@@@ loop_labels.ts @@@@@@ // tslint:disable diff --git a/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt b/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt index 091bed03655..24ae4f116cf 100644 --- a/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt +++ b/packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt @@ -560,6 +560,38 @@ import { Component, input } from '@angular/core'; export class InlineTmpl { justify = input<'start' | 'end'>('end'); } +@@@@@@ jit_true_component_external_tmpl.html @@@@@@ + +{{test()}} +@@@@@@ jit_true_components.ts @@@@@@ + +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +// tslint:disable + +import { Component, input } from '@angular/core'; + +@Component({ + jit: true, + template: '{{test()}}', +}) +class JitTrueComponent { + test = input(true); +} + +@Component({ + jit: true, + templateUrl: './jit_true_component_external_tmpl.html', +}) +class JitTrueComponentExternalTmpl { + test = input(true); +} @@@@@@ loop_labels.ts @@@@@@ // tslint:disable