fix(ngcc): fix compilation of ChangeDetectorRef in pipe constructors

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.

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', () => {

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', () => {

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', () => {

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', () => {

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

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: [],