From edf7f670f06da8dc7c6a7de8e6b9366e794bf0ea Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Thu, 18 Aug 2022 15:35:08 -0700 Subject: [PATCH] refactor(compiler): Add a new helper method `getOwningNgModule`. (#47166) This helper accepts a class for an Angular trait, and returns the NgModule which owns that trait. This will be useful for the language service import project, which needs to edit import arrays on the module. PR Close #47166 --- .../src/ngtsc/typecheck/api/checker.ts | 14 +- .../src/ngtsc/typecheck/src/checker.ts | 22 ++- .../typecheck/src/template_symbol_builder.ts | 1 + .../test/ngtsc/ls_typecheck_helpers_spec.ts | 141 ++++++++++++++---- 4 files changed, 142 insertions(+), 36 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index de67e9a1a87..4358c46240f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -151,6 +151,12 @@ export interface TemplateTypeChecker { */ getPrimaryAngularDecorator(target: ts.ClassDeclaration): ts.Decorator|null; + /** + * Get the class of the NgModule that owns this Angular trait. If the result is `null`, that + * probably means the provided component is standalone. + */ + getOwningNgModule(component: ts.ClassDeclaration): ts.ClassDeclaration|null; + /** * Retrieve any potential DOM bindings for the given element. * @@ -194,8 +200,8 @@ export interface TemplateTypeChecker { */ export enum OptimizeFor { /** - * Indicates that a consumer of a `TemplateTypeChecker` is only interested in results for a given - * file, and wants them as fast as possible. + * Indicates that a consumer of a `TemplateTypeChecker` is only interested in results for a + * given file, and wants them as fast as possible. * * Calling `TemplateTypeChecker` methods successively for multiple files while specifying * `OptimizeFor.SingleFile` can result in significant unnecessary overhead overall. @@ -203,8 +209,8 @@ export enum OptimizeFor { SingleFile, /** - * Indicates that a consumer of a `TemplateTypeChecker` intends to query for results pertaining to - * the entire user program, and so the type-checker should internally optimize for this case. + * Indicates that a consumer of a `TemplateTypeChecker` intends to query for results pertaining + * to the entire user program, and so the type-checker should internally optimize for this case. * * Initial calls to retrieve type-checking information may take longer, but repeated calls to * gather information for the whole user program will be significantly faster with this mode of diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 4e91016eda1..aa988ccc2d3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -615,9 +615,6 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { } getPrimaryAngularDecorator(target: ts.ClassDeclaration): ts.Decorator|null { - // TODO(dylhunn): It seems like we need to call this to make sure that - // `typeCheckAdapter.typeCheck()` is called, which in `ngtsc/typecheck/testing/index.ts:setup` - // actually populates the `metadataRegistry`, which is needed for the `metadataReader` to work. this.ensureAllShimsForOneFile(target.getSourceFile()); if (!isNamedClassDeclaration(target)) { @@ -642,6 +639,25 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { return null; } + getOwningNgModule(component: ts.ClassDeclaration): ts.ClassDeclaration|null { + if (!isNamedClassDeclaration(component)) { + return null; + } + + const dirMeta = this.metaReader.getDirectiveMetadata(new Reference(component)); + if (dirMeta !== null && dirMeta.isStandalone) { + return null; + } + + const scope = this.componentScopeReader.getScopeForComponent(component); + if (scope === null || scope.kind !== ComponentScopeKind.NgModule || + !isNamedClassDeclaration(scope.ngModule)) { + return null; + } + + return scope.ngModule; + } + private getScopeData(component: ts.ClassDeclaration): ScopeData|null { if (this.scopeCache.has(component)) { return this.scopeCache.get(component)!; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index 8261443f64f..bd62d6b2cd5 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -10,6 +10,7 @@ import {AST, ASTWithSource, BindingPipe, Call, ParseSourceSpan, PropertyRead, Pr import ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; +import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; import {ComponentScopeKind, ComponentScopeReader} from '../../scope'; import {isAssignment, isSymbolWithValueDeclaration} from '../../util/src/typescript'; diff --git a/packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts b/packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts index 38349b615c8..8b540d9a31d 100644 --- a/packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ls_typecheck_helpers_spec.ts @@ -30,19 +30,18 @@ runInEachFileSystem(() => { env.tsconfig({strictTemplates: true, _enableTemplateTypeChecker: true}); }); - describe('supports `getPrimaryAngularDecorator()` ', () => { it('for components', () => { env.write('test.ts', ` - import {Component} from '@angular/core'; - - @Component({ - standalone: true, - selector: 'test-cmp', - template: '
', - }) - export class TestCmp {} - `); + import {Component} from '@angular/core'; + + @Component({ + standalone: true, + selector: 'test-cmp', + template: '
', + }) + export class TestCmp {} + `); const {program, checker} = env.driveTemplateTypeChecker(); const sf = program.getSourceFile(_('/test.ts')); expect(sf).not.toBeNull(); @@ -52,15 +51,15 @@ runInEachFileSystem(() => { it('for pipes', () => { env.write('test.ts', ` - import {Pipe, PipeTransform} from '@angular/core'; - - @Pipe({name: 'expPipe'}) - export class ExpPipe implements PipeTransform { - transform(value: number, exponent = 1): number { - return Math.pow(value, exponent); - } - } - `); + import {Pipe, PipeTransform} from '@angular/core'; + + @Pipe({name: 'expPipe'}) + export class ExpPipe implements PipeTransform { + transform(value: number, exponent = 1): number { + return Math.pow(value, exponent); + } + } + `); const {program, checker} = env.driveTemplateTypeChecker(); const sf = program.getSourceFile(_('/test.ts')); expect(sf).not.toBeNull(); @@ -70,16 +69,16 @@ runInEachFileSystem(() => { it('for NgModules', () => { env.write('test.ts', ` - import {NgModule} from '@angular/core'; - - @NgModule({ - declarations: [], - imports: [], - providers: [], - bootstrap: [] - }) - export class AppModule {} - `); + import {NgModule} from '@angular/core'; + + @NgModule({ + declarations: [], + imports: [], + providers: [], + bootstrap: [] + }) + export class AppModule {} + `); const {program, checker} = env.driveTemplateTypeChecker(); const sf = program.getSourceFile(_('/test.ts')); expect(sf).not.toBeNull(); @@ -87,5 +86,89 @@ runInEachFileSystem(() => { expect(decorator?.getText()).toContain(`declarations: []`); }); }); + + describe('supports `getOwningNgModule()` ', () => { + it('for components', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @NgModule({ + declarations: [AppCmp], + imports: [], + providers: [], + bootstrap: [AppCmp] + }) + export class AppModule {} + + @Component({ + selector: 'app-cmp', + template: '
', + }) + export class AppCmp {} + `); + const {program, checker} = env.driveTemplateTypeChecker(); + const sf = program.getSourceFile(_('/test.ts')); + expect(sf).not.toBeNull(); + const ngModuleKnownClass = getClass(sf!, 'AppModule'); + expect(ngModuleKnownClass).not.toBeNull(); + const ngModuleRetrievedClass = checker.getOwningNgModule(getClass(sf!, 'AppCmp')); + expect(ngModuleRetrievedClass).toEqual(ngModuleKnownClass); + }); + + it('for standalone components (which should be null)', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + + @NgModule({ + declarations: [AppCmp], + imports: [], + providers: [], + bootstrap: [AppCmp] + }) + export class AppModule {} + + @Component({ + selector: 'app-cmp', + template: '
', + standalone: true, + }) + export class AppCmp {} + `); + const {program, checker} = env.driveTemplateTypeChecker(); + const sf = program.getSourceFile(_('/test.ts')); + expect(sf).not.toBeNull(); + const ngModuleKnownClass = getClass(sf!, 'AppModule'); + expect(ngModuleKnownClass).not.toBeNull(); + const ngModuleRetrievedClass = checker.getOwningNgModule(getClass(sf!, 'AppCmp')); + expect(ngModuleRetrievedClass).toBe(null); + }); + + it('for pipes', () => { + env.write('test.ts', ` + import {Component, NgModule, Pipe, PipeTransform} from '@angular/core'; + + @NgModule({ + declarations: [ExpPipe], + imports: [], + providers: [], + }) + export class PipeModule {} + + @Pipe({name: 'expPipe'}) + export class ExpPipe implements PipeTransform { + transform(value: number, exponent = 1): number { + return Math.pow(value, exponent); + } + } + `); + const {program, checker} = env.driveTemplateTypeChecker(); + const sf = program.getSourceFile(_('/test.ts')); + expect(sf).not.toBeNull(); + const ngModuleKnownClass = getClass(sf!, 'PipeModule'); + expect(ngModuleKnownClass).not.toBeNull(); + const ngModuleRetrievedClass = checker.getOwningNgModule(getClass(sf!, 'ExpPipe')); + expect(ngModuleRetrievedClass).toEqual(ngModuleKnownClass); + }); + }); }); });