[17.3.x]: Patch port of incremental type-check w/ import manager fix

package.json

@@ -118,7 +118,7 @@
     "karma-jasmine": "^5.0.0",
     "karma-requirejs": "^1.1.0",
     "karma-sourcemap-loader": "^0.4.0",
-    "magic-string": "0.30.7",
+    "magic-string": "^0.30.8",
     "memo-decorator": "^2.0.1",
     "ngx-flamegraph": "0.0.12",
     "ngx-progressbar": "^11.1.0",

packages/compiler-cli/linker/src/file_linker/emit_scopes/emit_scope.ts

@@ -36,15 +36,15 @@ export class EmitScope<TStatement, TExpression> {
    */
   translateDefinition(definition: LinkedDefinition): TExpression {
     const expression = this.translator.translateExpression(
-        definition.expression, new LinkerImportGenerator(this.ngImport));
+        definition.expression, new LinkerImportGenerator(this.factory, this.ngImport));
 
     if (definition.statements.length > 0) {
       // Definition statements must be emitted "after" the declaration for which the definition is
       // being emitted. However, the linker only transforms individual declaration calls, and can't
       // insert statements after definitions. To work around this, the linker transforms the
       // definition into an IIFE which executes the definition statements before returning the
       // definition expression.
-      const importGenerator = new LinkerImportGenerator(this.ngImport);
+      const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport);
       return this.wrapInIifeWithStatements(
           expression,
           definition.statements.map(
@@ -59,7 +59,7 @@ export class EmitScope<TStatement, TExpression> {
    * Return any constant statements that are shared between all uses of this `EmitScope`.
    */
   getConstantStatements(): TStatement[] {
-    const importGenerator = new LinkerImportGenerator(this.ngImport);
+    const importGenerator = new LinkerImportGenerator(this.factory, this.ngImport);
     return this.constantPool.statements.map(
         statement => this.translator.translateStatement(statement, importGenerator));
   }

packages/compiler-cli/linker/src/file_linker/translator.ts

@@ -20,21 +20,23 @@ export class Translator<TStatement, TExpression> {
    * Translate the given output AST in the context of an expression.
    */
   translateExpression(
-      expression: o.Expression, imports: ImportGenerator<TExpression>,
+      expression: o.Expression, imports: ImportGenerator<null, TExpression>,
       options: TranslatorOptions<TExpression> = {}): TExpression {
     return expression.visitExpression(
-        new ExpressionTranslatorVisitor<TStatement, TExpression>(this.factory, imports, options),
+        new ExpressionTranslatorVisitor<null, TStatement, TExpression>(
+            this.factory, imports, null, options),
         new Context(false));
   }
 
   /**
    * Translate the given output AST in the context of a statement.
    */
   translateStatement(
-      statement: o.Statement, imports: ImportGenerator<TExpression>,
+      statement: o.Statement, imports: ImportGenerator<null, TExpression>,
       options: TranslatorOptions<TExpression> = {}): TStatement {
     return statement.visitStatement(
-        new ExpressionTranslatorVisitor<TStatement, TExpression>(this.factory, imports, options),
+        new ExpressionTranslatorVisitor<null, TStatement, TExpression>(
+            this.factory, imports, null, options),
         new Context(true));
   }
 }

packages/compiler-cli/linker/src/linker_import_generator.ts

@@ -5,7 +5,7 @@
  * 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 {ImportGenerator, NamedImport} from '../../src/ngtsc/translator';
+import {AstFactory, ImportGenerator, ImportRequest} from '../../src/ngtsc/translator';
 
 import {FatalLinkerError} from './fatal_linker_error';
 
@@ -17,17 +17,19 @@ import {FatalLinkerError} from './fatal_linker_error';
  * must be achieved by property access on an `ng` namespace identifier, which is passed in via the
  * constructor.
  */
-export class LinkerImportGenerator<TExpression> implements ImportGenerator<TExpression> {
-  constructor(private ngImport: TExpression) {}
-
-  generateNamespaceImport(moduleName: string): TExpression {
-    this.assertModuleName(moduleName);
-    return this.ngImport;
+export class LinkerImportGenerator<TStatement, TExpression> implements
+    ImportGenerator<null, TExpression> {
+  constructor(private factory: AstFactory<TStatement, TExpression>, private ngImport: TExpression) {
   }
 
-  generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<TExpression> {
-    this.assertModuleName(moduleName);
-    return {moduleImport: this.ngImport, symbol: originalSymbol};
+  addImport(request: ImportRequest<null>): TExpression {
+    this.assertModuleName(request.exportModuleSpecifier);
+
+    if (request.exportSymbolName === null) {
+      return this.ngImport;
+    }
+
+    return this.factory.createPropertyAccess(this.ngImport, request.exportSymbolName);
   }
 
   private assertModuleName(moduleName: string): void {

packages/compiler-cli/linker/test/file_linker/translator_spec.ts

@@ -6,21 +6,30 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as o from '@angular/compiler';
-import {ImportGenerator, NamedImport, TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator';
+import {TypeScriptAstFactory} from '@angular/compiler-cli/src/ngtsc/translator';
 import ts from 'typescript';
 
 import {Translator} from '../../src/file_linker/translator';
+import {LinkerImportGenerator} from '../../src/linker_import_generator';
+
 import {generate} from './helpers';
 
+const ngImport = ts.factory.createIdentifier('ngImport');
+
 describe('Translator', () => {
   let factory: TypeScriptAstFactory;
-  beforeEach(() => factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false));
+  let importGenerator: LinkerImportGenerator<ts.Statement, ts.Expression>;
+
+  beforeEach(() => {
+    factory = new TypeScriptAstFactory(/* annotateForClosureCompiler */ false);
+    importGenerator = new LinkerImportGenerator<ts.Statement, ts.Expression>(factory, ngImport);
+  });
 
   describe('translateExpression()', () => {
     it('should generate expression specific output', () => {
       const translator = new Translator<ts.Statement, ts.Expression>(factory);
       const outputAst = new o.WriteVarExpr('foo', new o.LiteralExpr(42));
-      const translated = translator.translateExpression(outputAst, new MockImportGenerator());
+      const translated = translator.translateExpression(outputAst, importGenerator);
       expect(generate(translated)).toEqual('(foo = 42)');
     });
   });
@@ -29,19 +38,8 @@ describe('Translator', () => {
     it('should generate statement specific output', () => {
       const translator = new Translator<ts.Statement, ts.Expression>(factory);
       const outputAst = new o.ExpressionStatement(new o.WriteVarExpr('foo', new o.LiteralExpr(42)));
-      const translated = translator.translateStatement(outputAst, new MockImportGenerator());
+      const translated = translator.translateStatement(outputAst, importGenerator);
       expect(generate(translated)).toEqual('foo = 42;');
     });
   });
-  class MockImportGenerator implements ImportGenerator<ts.Expression> {
-    generateNamespaceImport(moduleName: string): ts.Expression {
-      return factory.createLiteral(moduleName);
-    }
-    generateNamedImport(moduleName: string, originalSymbol: string): NamedImport<ts.Expression> {
-      return {
-        moduleImport: factory.createLiteral(moduleName),
-        symbol: originalSymbol,
-      };
-    }
-  }
 });

packages/compiler-cli/linker/test/linker_import_generator_spec.ts

@@ -5,37 +5,62 @@
  * 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 {TypeScriptAstFactory} from '../../src/ngtsc/translator';
 import {LinkerImportGenerator} from '../src/linker_import_generator';
 
-const ngImport = {
-  ngImport: true
-};
+const ngImport = ts.factory.createIdentifier('ngImport');
 
 describe('LinkerImportGenerator<TExpression>', () => {
   describe('generateNamespaceImport()', () => {
     it('should error if the import is not `@angular/core`', () => {
-      const generator = new LinkerImportGenerator(ngImport);
-      expect(() => generator.generateNamespaceImport('other/import'))
+      const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
+          new TypeScriptAstFactory(false), ngImport);
+
+      expect(
+          () => generator.addImport(
+              {exportModuleSpecifier: 'other/import', exportSymbolName: null, requestedFile: null}))
           .toThrowError(`Unable to import from anything other than '@angular/core'`);
     });
 
     it('should return the ngImport expression for `@angular/core`', () => {
-      const generator = new LinkerImportGenerator(ngImport);
-      expect(generator.generateNamespaceImport('@angular/core')).toBe(ngImport);
+      const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
+          new TypeScriptAstFactory(false), ngImport);
+
+      expect(generator.addImport({
+        exportModuleSpecifier: '@angular/core',
+        exportSymbolName: null,
+        requestedFile: null
+      })).toBe(ngImport);
     });
   });
 
   describe('generateNamedImport()', () => {
     it('should error if the import is not `@angular/core`', () => {
-      const generator = new LinkerImportGenerator(ngImport);
-      expect(() => generator.generateNamedImport('other/import', 'someSymbol'))
-          .toThrowError(`Unable to import from anything other than '@angular/core'`);
+      const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
+          new TypeScriptAstFactory(false), ngImport);
+
+      expect(() => generator.addImport({
+        exportModuleSpecifier: 'other/import',
+        exportSymbolName: 'someSymbol',
+        requestedFile: null,
+      })).toThrowError(`Unable to import from anything other than '@angular/core'`);
     });
 
     it('should return a `NamedImport` object containing the ngImport expression', () => {
-      const generator = new LinkerImportGenerator(ngImport);
-      expect(generator.generateNamedImport('@angular/core', 'someSymbol'))
-          .toEqual({moduleImport: ngImport, symbol: 'someSymbol'});
+      const generator = new LinkerImportGenerator<ts.Statement, ts.Expression>(
+          new TypeScriptAstFactory(false), ngImport);
+
+      const result = generator.addImport({
+        exportModuleSpecifier: '@angular/core',
+        exportSymbolName: 'someSymbol',
+        requestedFile: null,
+      });
+
+      expect(ts.isPropertyAccessExpression(result)).toBe(true);
+      expect((result as ts.PropertyAccessExpression).name.text).toBe('someSymbol');
+      expect((result as ts.PropertyAccessExpression).expression).toBe(ngImport);
     });
   });
 });

packages/compiler-cli/src/ngtsc/annotations/common/test/metadata_spec.ts

@@ -10,10 +10,9 @@ import ts from 'typescript';
 
 import {absoluteFrom, getSourceFileOrError} from '../../../file_system';
 import {runInEachFileSystem, TestFile} from '../../../file_system/testing';
-import {NoopImportRewriter} from '../../../imports';
 import {TypeScriptReflectionHost} from '../../../reflection';
 import {getDeclaration, makeProgram} from '../../../testing';
-import {ImportManager, translateStatement} from '../../../translator';
+import {ImportManager, presetImportManagerForceNamespaceImports, translateStatement} from '../../../translator';
 import {extractClassMetadata} from '../src/metadata';
 
 runInEachFileSystem(() => {
@@ -133,9 +132,9 @@ runInEachFileSystem(() => {
       return '';
     }
     const sf = getSourceFileOrError(program, _('/index.ts'));
-    const im = new ImportManager(new NoopImportRewriter(), 'i');
+    const im = new ImportManager(presetImportManagerForceNamespaceImports);
     const stmt = compileClassMetadata(call).toStmt();
-    const tsStatement = translateStatement(stmt, im);
+    const tsStatement = translateStatement(sf, stmt, im);
     const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tsStatement, sf);
     return res.replace(/\s+/g, ' ');
   }

packages/compiler-cli/src/ngtsc/imports/index.ts

@@ -13,7 +13,7 @@ export {DeferredSymbolTracker} from './src/deferred_symbol_tracker';
 export {AbsoluteModuleStrategy, assertSuccessfulReferenceEmit, EmittedReference, FailedEmitResult, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitKind, ReferenceEmitResult, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter';
 export {ImportedSymbolsTracker} from './src/imported_symbols_tracker';
 export {LocalCompilationExtraImportsTracker} from './src/local_compilation_extra_imports_tracker';
-export {isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './src/patch_alias_reference_resolution';
+export {AliasImportDeclaration, isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './src/patch_alias_reference_resolution';
 export {Reexport} from './src/reexport';
 export {OwningModule, Reference} from './src/references';
 export {ModuleResolver} from './src/resolver';

packages/compiler-cli/src/ngtsc/imports/src/core.ts

@@ -12,14 +12,6 @@ import {relativePathBetween} from '../../util/src/path';
  * Rewrites imports of symbols being written into generated code.
  */
 export interface ImportRewriter {
-  /**
-   * Should the given symbol be imported at all?
-   *
-   * If `true`, the symbol should be imported from the given specifier. If `false`, the symbol
-   * should be referenced directly, without an import.
-   */
-  shouldImportSymbol(symbol: string, specifier: string): boolean;
-
   /**
    * Optionally rewrite a reference to an imported symbol, changing either the binding prefix or the
    * symbol name itself.
@@ -36,10 +28,6 @@ export interface ImportRewriter {
  * `ImportRewriter` that does no rewriting.
  */
 export class NoopImportRewriter implements ImportRewriter {
-  shouldImportSymbol(symbol: string, specifier: string): boolean {
-    return true;
-  }
-
   rewriteSymbol(symbol: string, specifier: string): string {
     return symbol;
   }
@@ -78,10 +66,6 @@ const CORE_MODULE = '@angular/core';
 export class R3SymbolsImportRewriter implements ImportRewriter {
   constructor(private r3SymbolsPath: string) {}
 
-  shouldImportSymbol(symbol: string, specifier: string): boolean {
-    return true;
-  }
-
   rewriteSymbol(symbol: string, specifier: string): string {
     if (specifier !== CORE_MODULE) {
       // This import isn't from core, so ignore it.

packages/compiler-cli/src/ngtsc/imports/src/patch_alias_reference_resolution.ts

@@ -8,6 +8,9 @@
 
 import ts from 'typescript';
 
+/** Possible alias import declarations */
+export type AliasImportDeclaration = ts.ImportSpecifier|ts.NamespaceImport|ts.ImportClause;
+
 /**
  * Describes a TypeScript transformation context with the internal emit
  * resolver exposed. There are requests upstream in TypeScript to expose
@@ -22,7 +25,7 @@ const patchedReferencedAliasesSymbol = Symbol('patchedReferencedAliases');
 /** Describes a subset of the TypeScript internal emit resolver. */
 interface EmitResolver {
   isReferencedAliasDeclaration?(node: ts.Node, ...args: unknown[]): void;
-  [patchedReferencedAliasesSymbol]?: Set<ts.Declaration>;
+  [patchedReferencedAliasesSymbol]?: Set<AliasImportDeclaration>;
 }
 
 /**
@@ -95,9 +98,9 @@ export function loadIsReferencedAliasDeclarationPatch(context: ts.Transformation
     throwIncompatibleTransformationContextError();
   }
 
-  const referencedAliases = new Set<ts.Declaration>();
+  const referencedAliases = new Set<AliasImportDeclaration>();
   emitResolver.isReferencedAliasDeclaration = function(node, ...args) {
-    if (isAliasImportDeclaration(node) && referencedAliases.has(node)) {
+    if (isAliasImportDeclaration(node) && (referencedAliases as Set<ts.Node>).has(node)) {
       return true;
     }
     return originalIsReferencedAliasDeclaration.call(emitResolver, node, ...args);
@@ -110,8 +113,7 @@ export function loadIsReferencedAliasDeclarationPatch(context: ts.Transformation
  * declarations can be import specifiers, namespace imports or import clauses
  * as these do not declare an actual symbol but just point to a target declaration.
  */
-export function isAliasImportDeclaration(node: ts.Node): node is ts.ImportSpecifier|
-    ts.NamespaceImport|ts.ImportClause {
+export function isAliasImportDeclaration(node: ts.Node): node is AliasImportDeclaration {
   return ts.isImportSpecifier(node) || ts.isNamespaceImport(node) || ts.isImportClause(node);
 }
 

packages/compiler-cli/src/ngtsc/transform/index.ts

@@ -12,4 +12,3 @@ export {ClassRecord, TraitCompiler} from './src/compilation';
 export {declarationTransformFactory, DtsTransformRegistry, IvyDeclarationDtsTransform} from './src/declaration';
 export {AnalyzedTrait, PendingTrait, ResolvedTrait, SkippedTrait, Trait, TraitState} from './src/trait';
 export {ivyTransformFactory} from './src/transform';
-export {addImports} from './src/utils';