From 989bfd2e972b19668723a66dd326d50eb9d81629 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Sun, 28 Apr 2019 20:48:34 +0100 Subject: [PATCH] fix(ivy): ngcc - support namespaced identifiers (#25445) In UMD formats, imports are always namespaced. This commit makes ngcc more tolerant of such structures. PR Close #25445 --- .../ngcc/src/host/esm2015_host.ts | 84 ++++++++++++------- .../ngcc/test/host/esm2015_host_spec.ts | 39 +++++++++ .../ngcc/test/host/esm5_host_spec.ts | 45 ++++++++++ 3 files changed, 137 insertions(+), 31 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 3758af6851..e2d666773e 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -305,19 +305,6 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N return null; } - /** - * Determine if an identifier was imported from another module and return `Import` metadata - * describing its origin. - * - * @param id a TypeScript `ts.Identifer` to reflect. - * - * @returns metadata about the `Import` if the identifier was imported from another module, or - * `null` if the identifier doesn't resolve to an import but instead is locally defined. - */ - getImportOfIdentifier(id: ts.Identifier): Import|null { - return super.getImportOfIdentifier(id) || this.getImportOfNamespacedIdentifier(id); - } - /** * Find all the classes that contain decorations in a given file. * @param sourceFile The source file to search for decorated classes. @@ -877,14 +864,20 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N const decorator = reflectObjectLiteral(node); // Is the value of the `type` property an identifier? - const typeIdentifier = decorator.get('type'); - if (typeIdentifier && ts.isIdentifier(typeIdentifier)) { - decorators.push({ - name: typeIdentifier.text, - identifier: typeIdentifier, - import: this.getImportOfIdentifier(typeIdentifier), node, - args: getDecoratorArgs(node), - }); + let typeIdentifier = decorator.get('type'); + if (typeIdentifier) { + if (ts.isPropertyAccessExpression(typeIdentifier)) { + // the type is in a namespace, e.g. `core.Directive` + typeIdentifier = typeIdentifier.name; + } + if (ts.isIdentifier(typeIdentifier)) { + decorators.push({ + name: typeIdentifier.text, + identifier: typeIdentifier, + import: this.getImportOfIdentifier(typeIdentifier), node, + args: getDecoratorArgs(node), + }); + } } } }); @@ -1327,27 +1320,56 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N prop => !!prop.name && ts.isIdentifier(prop.name) && prop.name.text === 'ngModule') || null; - const ngModuleIdentifier = ngModuleProperty && ts.isPropertyAssignment(ngModuleProperty) && - ts.isIdentifier(ngModuleProperty.initializer) && ngModuleProperty.initializer || - null; - // If no `ngModule` property was found in an object literal return value, return `null` to - // indicate that the provided node does not appear to be a `ModuleWithProviders` function. - if (ngModuleIdentifier === null) { + if (!ngModuleProperty || !ts.isPropertyAssignment(ngModuleProperty)) { return null; } - const ngModuleDeclaration = this.getDeclarationOfIdentifier(ngModuleIdentifier); + // The ngModuleValue could be of the form `SomeModule` or `namespace_1.SomeModule` + const ngModuleValue = ngModuleProperty.initializer; + if (!ts.isIdentifier(ngModuleValue) && !ts.isPropertyAccessExpression(ngModuleValue)) { + return null; + } + + const ngModuleDeclaration = this.getDeclarationOfExpression(ngModuleValue); if (!ngModuleDeclaration) { throw new Error( - `Cannot find a declaration for NgModule ${ngModuleIdentifier.text} referenced in "${declaration!.getText()}"`); + `Cannot find a declaration for NgModule ${ngModuleValue.getText()} referenced in "${declaration!.getText()}"`); } if (!hasNameIdentifier(ngModuleDeclaration.node)) { return null; } - const ngModule = ngModuleDeclaration as Declaration; + return { + name, + ngModule: ngModuleDeclaration as Declaration, declaration, container + }; + } - return {name, ngModule, declaration, container}; + protected getDeclarationOfExpression(expression: ts.Expression): Declaration|null { + if (ts.isIdentifier(expression)) { + return this.getDeclarationOfIdentifier(expression); + } + + if (!ts.isPropertyAccessExpression(expression) || !ts.isIdentifier(expression.expression)) { + return null; + } + + const namespaceDecl = this.getDeclarationOfIdentifier(expression.expression); + if (!namespaceDecl || !ts.isSourceFile(namespaceDecl.node)) { + return null; + } + + const namespaceExports = this.getExportsOfModule(namespaceDecl.node); + if (namespaceExports === null) { + return null; + } + + if (!namespaceExports.has(expression.name.text)) { + return null; + } + + const exportDecl = namespaceExports.get(expression.name.text) !; + return {...exportDecl, viaModule: namespaceDecl.viaModule}; } } diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts index 98e20267ff..1da2f5b0eb 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -521,6 +521,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ name: '/src/functions.js', contents: ` import {ExternalModule} from './module'; + import * as mod from './module'; export class SomeService {} export class InternalModule {} export function aNumber() { return 42; } @@ -534,12 +535,14 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ export function ngModuleString() { return { ngModule: 'foo' }; } export function ngModuleObject() { return { ngModule: { foo: 42 } }; } export function externalNgModule() { return { ngModule: ExternalModule }; } + export function namespacedExternalNgModule() { return { ngModule: mod.ExternalModule }; } ` }, { name: '/src/methods.js', contents: ` import {ExternalModule} from './module'; + import * as mod from './module'; export class SomeService {} export class InternalModule { static aNumber() { return 42; } @@ -553,11 +556,13 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ static ngModuleString() { return { ngModule: 'foo' }; } static ngModuleObject() { return { ngModule: { foo: 42 } }; } static externalNgModule() { return { ngModule: ExternalModule }; } + static namespacedExternalNgModule() { return { ngModule: mod.ExternalModule }; } instanceNgModuleIdentifier() { return { ngModule: InternalModule }; } instanceNgModuleWithEmptyProviders() { return { ngModule: InternalModule, providers: [] }; } instanceNgModuleWithProviders() { return { ngModule: InternalModule, providers: [SomeService] }; } instanceExternalNgModule() { return { ngModule: ExternalModule }; } + instanceNamespacedExternalNgModule() { return { ngModule: mod.ExternalModule }; } } ` }, @@ -574,6 +579,19 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ {name: '/src/module.js', contents: 'export class ExternalModule {}'}, ]; +const NAMESPACED_IMPORT_FILE = { + name: '/some_directive.js', + contents: ` + import * as core from '@angular/core'; + + class SomeDirective { + } + SomeDirective.decorators = [ + { type: core.Directive, args: [{ selector: '[someDirective]' },] } + ]; + ` +}; + describe('Esm2015ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -1348,6 +1366,25 @@ describe('Esm2015ReflectionHost', () => { expect(actualDeclaration !.viaModule).toBe('@angular/core'); }); + it('should return the source-file of an import namespace', () => { + const {program} = makeTestBundleProgram([NAMESPACED_IMPORT_FILE]); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration( + program, NAMESPACED_IMPORT_FILE.name, 'SomeDirective', ts.isClassDeclaration); + const classDecorators = host.getDecoratorsOfDeclaration(classNode) !; + const identifier = (((classDecorators[0].node as ts.ObjectLiteralExpression) + .properties[0] as ts.PropertyAssignment) + .initializer as ts.PropertyAccessExpression) + .expression as ts.Identifier; + + const expectedDeclarationNode = + program.getSourceFile('node_modules/@angular/core/index.d.ts') !; + const actualDeclaration = host.getDeclarationOfIdentifier(identifier); + expect(actualDeclaration).not.toBe(null); + expect(actualDeclaration !.node).toBe(expectedDeclarationNode); + expect(actualDeclaration !.viaModule).toBe(null); + }); + it('should return the original declaration of an aliased class', () => { const program = makeTestProgram(CLASS_EXPRESSION_FILE); const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); @@ -1649,6 +1686,7 @@ describe('Esm2015ReflectionHost', () => { ['ngModuleWithEmptyProviders', 'InternalModule'], ['ngModuleWithProviders', 'InternalModule'], ['externalNgModule', 'ExternalModule'], + ['namespacedExternalNgModule', 'ExternalModule'], ]); }); @@ -1665,6 +1703,7 @@ describe('Esm2015ReflectionHost', () => { ['ngModuleWithEmptyProviders', 'InternalModule'], ['ngModuleWithProviders', 'InternalModule'], ['externalNgModule', 'ExternalModule'], + ['namespacedExternalNgModule', 'ExternalModule'], ]); }); diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index 9c38c7f4fb..c4966b9023 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -643,6 +643,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ name: '/src/functions.js', contents: ` import {ExternalModule} from './module'; + import * as mod from './module'; var SomeService = (function() { function SomeService() {} @@ -664,6 +665,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ export function ngModuleString() { return { ngModule: 'foo' }; } export function ngModuleObject() { return { ngModule: { foo: 42 } }; } export function externalNgModule() { return { ngModule: ExternalModule }; } + export function namespacedExternalNgModule() { return { ngModule: mod.ExternalModule }; } export {SomeService, InternalModule}; ` }, @@ -671,6 +673,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ name: '/src/methods.js', contents: ` import {ExternalModule} from './module'; + import * as mod from './module'; var SomeService = (function() { function SomeService() {} return SomeService; @@ -683,6 +686,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ instanceNgModuleWithEmptyProviders: function() { return { ngModule: InternalModule, providers: [] }; }, instanceNgModuleWithProviders: function() { return { ngModule: InternalModule, providers: [SomeService] }; }, instanceExternalNgModule: function() { return { ngModule: ExternalModule }; }, + namespacedExternalNgModule = function() { return { ngModule: mod.ExternalModule }; }, }; InternalModule.aNumber = function() { return 42; }; InternalModule.aString = function() { return 'foo'; }; @@ -695,6 +699,7 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ InternalModule.ngModuleString = function() { return { ngModule: 'foo' }; }; InternalModule.ngModuleObject = function() { return { ngModule: { foo: 42 } }; }; InternalModule.externalNgModule = function() { return { ngModule: ExternalModule }; }; + InternalModule.namespacedExternalNgModule = function() { return { ngModule: mod.ExternalModule }; }; return InternalModule; }()); export {SomeService, InternalModule}; @@ -716,6 +721,22 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ {name: '/src/module.js', contents: 'export class ExternalModule {}'}, ]; +const NAMESPACED_IMPORT_FILE = { + name: '/some_directive.js', + contents: ` + import * as core from '@angular/core'; + + var SomeDirective = (function() { + function SomeDirective() { + } + SomeDirective.decorators = [ + { type: core.Directive, args: [{ selector: '[someDirective]' },] } + ]; + return SomeDirective; + }()); + ` +}; + describe('Esm5ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -1500,6 +1521,25 @@ describe('Esm5ReflectionHost', () => { expect(actualDeclaration !.viaModule).toBe('@angular/core'); }); + it('should return the source-file of an import namespace', () => { + const program = makeTestProgram(NAMESPACED_IMPORT_FILE); + const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration( + program, NAMESPACED_IMPORT_FILE.name, 'SomeDirective', isNamedVariableDeclaration); + const classDecorators = host.getDecoratorsOfDeclaration(classNode) !; + const identifier = (((classDecorators[0].node as ts.ObjectLiteralExpression) + .properties[0] as ts.PropertyAssignment) + .initializer as ts.PropertyAccessExpression) + .expression as ts.Identifier; + + const expectedDeclarationNode = + program.getSourceFile('node_modules/@angular/core/index.d.ts') !; + const actualDeclaration = host.getDeclarationOfIdentifier(identifier); + expect(actualDeclaration).not.toBe(null); + expect(actualDeclaration !.node).toBe(expectedDeclarationNode); + expect(actualDeclaration !.viaModule).toBe(null); + }); + it('should return the correct declaration for an inner function identifier inside an ES5 IIFE', () => { const superGetDeclarationOfIdentifierSpy = @@ -1851,6 +1891,7 @@ describe('Esm5ReflectionHost', () => { ['ngModuleWithEmptyProviders', 'InternalModule'], ['ngModuleWithProviders', 'InternalModule'], ['externalNgModule', 'ExternalModule'], + ['namespacedExternalNgModule', 'ExternalModule'], ]); }); @@ -1877,6 +1918,10 @@ describe('Esm5ReflectionHost', () => { 'function() { return { ngModule: ExternalModule }; }', 'ExternalModule', ], + [ + 'function() { return { ngModule: mod.ExternalModule }; }', + 'ExternalModule', + ], ]); });