refactor(compiler-cli): use a transformer for dts files (#28342)

The current DtsFileTransformer works by intercepting file writes and editing the source string directly.

This PR refactors it as a afterDeclaration transform in order to fit better in the TypeScript API.

This is part of a greater effort of converting ngtsc to be usable as a TS transform plugin.

PR Close #28342

packages/compiler-cli/src/ngtsc/program.ts

@@ -24,7 +24,7 @@ import {HostResourceLoader} from './resource_loader';
 import {NgModuleRouteAnalyzer} from './routing';
 import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator, SummaryGenerator, generatedFactoryTransform} from './shims';
 import {ivySwitchTransform} from './switch';
-import {IvyCompilation, ivyTransformFactory} from './transform';
+import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform';
 import {TypeCheckContext, TypeCheckProgramHost} from './typecheck';
 import {normalizeSeparators} from './util/src/path';
 import {isDtsPath} from './util/src/typescript';
@@ -231,17 +231,13 @@ export class NgtscProgram implements api.Program {
   }): ts.EmitResult {
     const emitCallback = opts && opts.emitCallback || defaultEmitCallback;
 
-    this.ensureAnalyzed();
+    const compilation = this.ensureAnalyzed();
 
-    // Since there is no .d.ts transformation API, .d.ts files are transformed during write.
     const writeFile: ts.WriteFileCallback =
         (fileName: string, data: string, writeByteOrderMark: boolean,
          onError: ((message: string) => void) | undefined,
          sourceFiles: ReadonlyArray<ts.SourceFile>) => {
-          if (fileName.endsWith('.d.ts')) {
-            data = sourceFiles.reduce(
-                (data, sf) => this.compilation !.transformedDtsFor(sf.fileName, data), data);
-          } else if (this.closureCompilerEnabled && fileName.endsWith('.js')) {
+          if (this.closureCompilerEnabled && fileName.endsWith('.js')) {
             data = nocollapseHack(data);
           }
           this.host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles);
@@ -249,7 +245,8 @@ export class NgtscProgram implements api.Program {
 
     const customTransforms = opts && opts.customTransformers;
     const beforeTransforms =
-        [ivyTransformFactory(this.compilation !, this.reflector, this.importRewriter, this.isCore)];
+        [ivyTransformFactory(compilation, this.reflector, this.importRewriter, this.isCore)];
+    const afterDeclarationsTransforms = [declarationTransformFactory(compilation)];
 
     if (this.factoryToSourceInfo !== null) {
       beforeTransforms.push(
@@ -271,6 +268,7 @@ export class NgtscProgram implements api.Program {
       customTransformers: {
         before: beforeTransforms,
         after: customTransforms && customTransforms.afterTs,
+        afterDeclarations: afterDeclarationsTransforms,
       },
     });
     return emitResult;

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

@@ -8,5 +8,5 @@
 
 export * from './src/api';
 export {IvyCompilation} from './src/compilation';
-export {DtsFileTransformer} from './src/declaration';
+export {DtsFileTransformer, declarationTransformFactory} from './src/declaration';
 export {ivyTransformFactory} from './src/transform';

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

@@ -220,17 +220,21 @@ export class IvyCompilation {
   }
 
   /**
-   * Process a .d.ts source string and return a transformed version that incorporates the changes
+   * Process a declaration file and return a transformed version that incorporates the changes
    * made to the source file.
    */
-  transformedDtsFor(tsFileName: string, dtsOriginalSource: string): string {
-    // No need to transform if no changes have been requested to the input file.
-    if (!this.dtsMap.has(tsFileName)) {
-      return dtsOriginalSource;
+  transformedDtsFor(file: ts.SourceFile, context: ts.TransformationContext): ts.SourceFile {
+    // No need to transform if it's not a declarations file, or if no changes have been requested
+    // to the input file.
+    // Due to the way TypeScript afterDeclarations transformers work, the SourceFile path is the
+    // same as the original .ts.
+    // The only way we know it's actually a declaration file is via the isDeclarationFile property.
+    if (!file.isDeclarationFile || !this.dtsMap.has(file.fileName)) {
+      return file;
     }
 
-    // Return the transformed .d.ts source.
-    return this.dtsMap.get(tsFileName) !.transform(dtsOriginalSource, tsFileName);
+    // Return the transformed source.
+    return this.dtsMap.get(file.fileName) !.transform(file, context);
   }
 
   get diagnostics(): ReadonlyArray<ts.Diagnostic> { return this._diagnostics; }

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

@@ -12,9 +12,24 @@ import {ImportRewriter} from '../../imports';
 import {ImportManager, translateType} from '../../translator';
 
 import {CompileResult} from './api';
+import {IvyCompilation} from './compilation';
+import {addImports} from './utils';
 
 
 
+export function declarationTransformFactory(compilation: IvyCompilation):
+    ts.TransformerFactory<ts.Bundle|ts.SourceFile> {
+  return (context: ts.TransformationContext) => {
+    return (fileOrBundle) => {
+      if (ts.isBundle(fileOrBundle)) {
+        // Only attempt to transform source files.
+        return fileOrBundle;
+      }
+      return compilation.transformedDtsFor(fileOrBundle, context);
+    };
+  };
+}
+
 /**
  * Processes .d.ts file text and adds static field declarations, with types.
  */
@@ -32,36 +47,34 @@ export class DtsFileTransformer {
   recordStaticField(name: string, decls: CompileResult[]): void { this.ivyFields.set(name, decls); }
 
   /**
-   * Process the .d.ts text for a file and add any declarations which were recorded.
+   * Transform the declaration file and add any declarations which were recorded.
    */
-  transform(dts: string, tsPath: string): string {
-    const dtsFile =
-        ts.createSourceFile('out.d.ts', dts, ts.ScriptTarget.Latest, false, ts.ScriptKind.TS);
-
-    for (let i = dtsFile.statements.length - 1; i >= 0; i--) {
-      const stmt = dtsFile.statements[i];
-      if (ts.isClassDeclaration(stmt) && stmt.name !== undefined &&
-          this.ivyFields.has(stmt.name.text)) {
-        const decls = this.ivyFields.get(stmt.name.text) !;
-        const before = dts.substring(0, stmt.end - 1);
-        const after = dts.substring(stmt.end - 1);
-
-        dts = before +
-            decls
-                .map(decl => {
+  transform(file: ts.SourceFile, context: ts.TransformationContext): ts.SourceFile {
+    const visitor: ts.Visitor = (node: ts.Node): ts.VisitResult<ts.Node> => {
+      // This class declaration needs to have fields added to it.
+      if (ts.isClassDeclaration(node) && node.name !== undefined &&
+          this.ivyFields.has(node.name.text)) {
+        const decls = this.ivyFields.get(node.name.text) !;
+        const newMembers = decls.map(decl => {
+          const modifiers = [ts.createModifier(ts.SyntaxKind.StaticKeyword)];
           const type = translateType(decl.type, this.imports);
-                  return `    static ${decl.name}: ${type};\n`;
-                })
-                .join('') +
-            after;
-      }
+          const typeRef = ts.createTypeReferenceNode(ts.createIdentifier(type), undefined);
+          return ts.createProperty(undefined, modifiers, decl.name, undefined, typeRef, undefined);
+        });
+
+        return ts.updateClassDeclaration(
+            node, node.decorators, node.modifiers, node.name, node.typeParameters,
+            node.heritageClauses, [...node.members, ...newMembers]);
       }
 
-    const imports = this.imports.getAllImports(tsPath);
-    if (imports.length !== 0) {
-      dts = imports.map(i => `import * as ${i.as} from '${i.name}';\n`).join('') + dts;
-    }
+      // Otherwise return node as is.
+      return ts.visitEachChild(node, visitor, context);
+    };
 
-    return dts;
+    // Recursively scan through the AST and add all class members needed.
+    const sf = ts.visitNode(file, visitor);
+
+    // Add new imports for this file.
+    return addImports(this.imports, sf);
   }
 }

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

@@ -14,9 +14,9 @@ import { ImportManager } from '../../translator';
  * and before the module body.
  * Can optionally add extra statements (e.g. new constants) before the body as well.
  */
-export function addImports(importManager: ImportManager, sf: ts.SourceFile,
+export function addImports(
+    importManager: ImportManager, sf: ts.SourceFile,
     extraStatements: ts.Statement[] = []): ts.SourceFile {
-
   // Generate the import statements to prepend.
   const addedImports = importManager.getAllImports(sf.fileName).map(i => {
     return ts.createImportDeclaration(

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -449,8 +449,8 @@ describe('ngtsc behavioral tests', () => {
     const dtsContents = env.getContents('test.d.ts');
 
     expect(jsContents).toContain('import { Foo } from \'./foo\';');
-    expect(jsContents).not.toMatch(/as i[0-9] from '.\/foo'/);
-    expect(dtsContents).toContain('as i1 from \'./foo\';');
+    expect(jsContents).not.toMatch(/as i[0-9] from ".\/foo"/);
+    expect(dtsContents).toContain('as i1 from "./foo";');
   });
 
   it('should compile NgModules with references to absolute components', () => {
@@ -477,8 +477,8 @@ describe('ngtsc behavioral tests', () => {
     const dtsContents = env.getContents('test.d.ts');
 
     expect(jsContents).toContain('import { Foo } from \'foo\';');
-    expect(jsContents).not.toMatch(/as i[0-9] from 'foo'/);
-    expect(dtsContents).toContain('as i1 from \'foo\';');
+    expect(jsContents).not.toMatch(/as i[0-9] from "foo"/);
+    expect(dtsContents).toContain('as i1 from "foo";');
   });
 
   it('should compile Pipes without errors', () => {
@@ -603,7 +603,7 @@ describe('ngtsc behavioral tests', () => {
       expect(jsContents).toContain('imports: [[RouterModule.forRoot()]]');
 
       const dtsContents = env.getContents('test.d.ts');
-      expect(dtsContents).toContain(`import * as i1 from 'router';`);
+      expect(dtsContents).toContain(`import * as i1 from "router";`);
       expect(dtsContents)
           .toContain('i0.ɵNgModuleDefWithMeta<TestModule, never, [typeof i1.RouterModule], never>');
     });
@@ -639,7 +639,7 @@ describe('ngtsc behavioral tests', () => {
       expect(jsContents).toContain('imports: [[RouterModule.forRoot()]]');
 
       const dtsContents = env.getContents('test.d.ts');
-      expect(dtsContents).toContain(`import * as i1 from 'router';`);
+      expect(dtsContents).toContain(`import * as i1 from "router";`);
       expect(dtsContents)
           .toContain(
               'i0.ɵNgModuleDefWithMeta<TestModule, never, [typeof i1.InternalRouterModule], never>');
@@ -673,7 +673,7 @@ describe('ngtsc behavioral tests', () => {
        expect(jsContents).toContain('imports: [[RouterModule.forRoot()]]');
 
        const dtsContents = env.getContents('test.d.ts');
-       expect(dtsContents).toContain(`import * as i1 from 'router';`);
+       expect(dtsContents).toContain(`import * as i1 from "router";`);
        expect(dtsContents)
            .toContain(
                'i0.ɵNgModuleDefWithMeta<TestModule, never, [typeof i1.RouterModule], never>');