diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 343880920c4c9..3ce41f290e868 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {absoluteFromSourceFile} from '../../../src/ngtsc/file_system'; import {Logger} from '../../../src/ngtsc/logging'; -import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, Import, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection'; import {isWithinPackage} from '../analysis/util'; import {BundleProgram} from '../packages/bundle_program'; import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils'; @@ -1608,10 +1608,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N /** * Compute the `TypeValueReference` for the given `typeExpression`. * - * In ngcc, all the `typeExpression` are guaranteed to be "values" because it is working in JS and - * not TS. This means that the TS compiler is not going to remove the "type" import and so we can - * always use a LOCAL `TypeValueReference` kind, rather than trying to force an additional import - * for non-local expressions. + * Although `typeExpression` is a valid `ts.Expression` that could be emitted directly into the + * generated code, ngcc still needs to resolve the declaration and create an `IMPORTED` type + * value reference as the compiler has specialized handling for some symbols, for example + * `ChangeDetectorRef` from `@angular/core`. Such an `IMPORTED` type value reference will result + * in a newly generated namespace import, instead of emitting the original `typeExpression` as is. */ private typeToValue(typeExpression: ts.Expression|null): TypeValueReference { if (typeExpression === null) { @@ -1621,13 +1622,42 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N }; } + const imp = this.getImportOfExpression(typeExpression); + const decl = this.getDeclarationOfExpression(typeExpression); + if (imp === null || decl === null || decl.node === null) { + return { + kind: TypeValueReferenceKind.LOCAL, + expression: typeExpression, + defaultImportStatement: null, + }; + } + return { - kind: TypeValueReferenceKind.LOCAL, - expression: typeExpression, - defaultImportStatement: null, + kind: TypeValueReferenceKind.IMPORTED, + valueDeclaration: decl.node, + moduleName: imp.from, + importedName: imp.name, + nestedPath: null, }; } + /** + * Determines where the `expression` is imported from. + * + * @param expression the expression to determine the import details for. + * @returns the `Import` for the expression, or `null` if the expression is not imported or the + * expression syntax is not supported. + */ + private getImportOfExpression(expression: ts.Expression): Import|null { + if (ts.isIdentifier(expression)) { + return this.getImportOfIdentifier(expression); + } else if (ts.isPropertyAccessExpression(expression) && ts.isIdentifier(expression.name)) { + return this.getImportOfIdentifier(expression.name); + } else { + return null; + } + } + /** * Get the parameter type and decorators for the constructor of a class, * where the information is stored on a static property of the class. diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts index c7d0f243b0c3e..366c15ff6113a 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts @@ -1211,7 +1211,7 @@ exports.MissingClass2 = MissingClass2; }); describe('getConstructorParameters', () => { - it('should always specify LOCAL type value references for decorated constructor parameter types', + it('should retain imported name for type value references for decorated constructor parameter types', () => { const files = [ { @@ -1271,7 +1271,7 @@ exports.MissingClass2 = MissingClass2; expect(parameters.map(p => p.name)).toEqual(['arg1', 'arg2', 'arg3']); expectTypeValueReferencesForParameters( - parameters, ['shared.Baz', 'local.External', 'SameFile']); + parameters, ['Baz', 'External', 'SameFile'], ['shared-lib', './local', null]); }); it('should find the decorated constructor parameters', () => { 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 96182d3e4858c..a0be48dc70935 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -1140,7 +1140,7 @@ runInEachFileSystem(() => { }); describe('getConstructorParameters()', () => { - it('should always specify LOCAL type value references for decorated constructor parameter types', + it('should retain imported name for type value references for decorated constructor parameter types', () => { const files = [ { @@ -1188,7 +1188,8 @@ runInEachFileSystem(() => { const parameters = host.getConstructorParameters(classNode)!; expect(parameters.map(p => p.name)).toEqual(['arg1', 'arg2', 'arg3']); - expectTypeValueReferencesForParameters(parameters, ['Baz', 'External', 'SameFile']); + expectTypeValueReferencesForParameters( + parameters, ['Baz', 'External', 'SameFile'], ['shared-lib', './local', null]); }); it('should find the decorated constructor parameters', () => { @@ -1205,7 +1206,8 @@ runInEachFileSystem(() => { '_viewContainer', '_template', 'injected' ]); expectTypeValueReferencesForParameters( - parameters, ['ViewContainerRef', 'TemplateRef', null]); + parameters, ['ViewContainerRef', 'TemplateRef', null], + ['@angular/core', '@angular/core', null]); }); it('should accept `ctorParameters` as an array', () => { 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 dafb07947d6a3..e5e3f0c75de0c 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -1252,7 +1252,7 @@ runInEachFileSystem(() => { }); describe('getConstructorParameters()', () => { - it('should always specify LOCAL type value references for decorated constructor parameter types', + it('should retain imported name for type value references for decorated constructor parameter types', () => { const files = [ { @@ -1310,7 +1310,8 @@ runInEachFileSystem(() => { const parameters = host.getConstructorParameters(classNode)!; expect(parameters.map(p => p.name)).toEqual(['arg1', 'arg2', 'arg3']); - expectTypeValueReferencesForParameters(parameters, ['Baz', 'External', 'SameFile']); + expectTypeValueReferencesForParameters( + parameters, ['Baz', 'External', 'SameFile'], ['shared-lib', './local', null]); }); it('should find the decorated constructor parameters', () => { diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts index cf918fb43f533..25a40b8626712 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -1332,7 +1332,7 @@ runInEachFileSystem(() => { }); describe('getConstructorParameters', () => { - it('should always specify LOCAL type value references for decorated constructor parameter types', + it('should retain imported name for type value references for decorated constructor parameter types', () => { const files = [ { @@ -1369,7 +1369,7 @@ runInEachFileSystem(() => { name: _('/main.js'), contents: ` (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('shared-lib), require('./local')) : + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('shared-lib'), require('./local')) : typeof define === 'function' && define.amd ? define('main', ['exports', 'shared-lib', './local'], factory) : (factory(global.main, global.shared, global.local)); }(this, (function (exports, shared, local) { 'use strict'; @@ -1401,7 +1401,7 @@ runInEachFileSystem(() => { expect(parameters.map(p => p.name)).toEqual(['arg1', 'arg2', 'arg3']); expectTypeValueReferencesForParameters( - parameters, ['shared.Baz', 'local.External', 'SameFile']); + parameters, ['Baz', 'External', 'SameFile'], ['shared-lib', './local', null]); }); it('should find the decorated constructor parameters', () => { diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 16c7dc89351ab..7208abe4df885 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -576,6 +576,35 @@ runInEachFileSystem(() => { `TestClass.ɵprov = ɵngcc0.ɵɵdefineInjectable({`); }); + // https://github.com/angular/angular/issues/38883 + it('should recognize ChangeDetectorRef as special symbol for pipes', () => { + compileIntoFlatEs2015Package('test-package', { + '/index.ts': ` + import {ChangeDetectorRef, Pipe, PipeTransform} from '@angular/core'; + + @Pipe({ + name: 'myTestPipe' + }) + export class TestClass implements PipeTransform { + constructor(cdr: ChangeDetectorRef) {} + transform(value: any) { return value; } + } + `, + }); + + mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: 'test-package', + propertiesToConsider: ['esm2015'], + }); + + const jsContents = fs.readFile(_(`/node_modules/test-package/index.js`)); + expect(jsContents) + .toContain( + `TestClass.ɵfac = function TestClass_Factory(t) { ` + + `return new (t || TestClass)(ɵngcc0.ɵɵinjectPipeChangeDetectorRef()); };`); + }); + it('should use the correct type name in typings files when an export has a different name in source files', () => { // We need to make sure that changes to the typings files use the correct name diff --git a/packages/compiler-cli/ngcc/test/integration/util.ts b/packages/compiler-cli/ngcc/test/integration/util.ts index 6f0439e69b80b..e8ab286ee2422 100644 --- a/packages/compiler-cli/ngcc/test/integration/util.ts +++ b/packages/compiler-cli/ngcc/test/integration/util.ts @@ -99,7 +99,14 @@ function compileIntoFlatPackage( program.emit(); }; - emit({declaration: true, module: options.module, target: options.target, lib: []}); + emit({ + declaration: true, + emitDecoratorMetadata: true, + moduleResolution: ts.ModuleResolutionKind.NodeJs, + module: options.module, + target: options.target, + lib: [], + }); // Copy over the JS and .d.ts files, and add a .metadata.json for each .d.ts file. for (const file of rootNames) { @@ -152,7 +159,9 @@ export function compileIntoApf( compileFs.ensureDir(compileFs.resolve('esm2015')); emit({ declaration: true, + emitDecoratorMetadata: true, outDir: './esm2015', + moduleResolution: ts.ModuleResolutionKind.NodeJs, module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2015, lib: [], @@ -178,7 +187,9 @@ export function compileIntoApf( compileFs.ensureDir(compileFs.resolve('esm5')); emit({ declaration: false, + emitDecoratorMetadata: true, outDir: './esm5', + moduleResolution: ts.ModuleResolutionKind.NodeJs, module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES5, lib: [],