From d45d3a3ef96d179f36749720db811ed23d39b99f Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Thu, 24 Jan 2019 10:38:58 +0000 Subject: [PATCH] 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 | 14 ++-- .../compiler-cli/src/ngtsc/transform/index.ts | 2 +- .../src/ngtsc/transform/src/compilation.ts | 18 +++-- .../src/ngtsc/transform/src/declaration.ts | 65 +++++++++++-------- .../src/ngtsc/transform/src/utils.ts | 16 ++--- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 14 ++-- 6 files changed, 72 insertions(+), 57 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 69a683aa59..2f379e87fc 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/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) => { - 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; diff --git a/packages/compiler-cli/src/ngtsc/transform/index.ts b/packages/compiler-cli/src/ngtsc/transform/index.ts index d1cd4ded2a..e7a8a0f74e 100644 --- a/packages/compiler-cli/src/ngtsc/transform/index.ts +++ b/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'; diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index 6e277d8125..aff334317c 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/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 { return this._diagnostics; } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts index e053ccda80..59a2bdbebb 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts +++ b/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 { + 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); + transform(file: ts.SourceFile, context: ts.TransformationContext): ts.SourceFile { + const visitor: ts.Visitor = (node: ts.Node): ts.VisitResult => { + // 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); + const typeRef = ts.createTypeReferenceNode(ts.createIdentifier(type), undefined); + return ts.createProperty(undefined, modifiers, decl.name, undefined, typeRef, undefined); + }); - 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 => { - const type = translateType(decl.type, this.imports); - return ` static ${decl.name}: ${type};\n`; - }) - .join('') + - after; + 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); } } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts index e4c802db16..03158c34ea 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/utils.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/utils.ts @@ -7,22 +7,22 @@ */ import * as ts from 'typescript'; -import { ImportManager } from '../../translator'; +import {ImportManager} from '../../translator'; /** * Adds extra imports in the import manage for this source file, after the existing imports * 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( - undefined, undefined, - ts.createImportClause(undefined, ts.createNamespaceImport(ts.createIdentifier(i.as))), - ts.createLiteral(i.name)); + undefined, undefined, + ts.createImportClause(undefined, ts.createNamespaceImport(ts.createIdentifier(i.as))), + ts.createLiteral(i.name)); }); // Filter out the existing imports and the source file body. All new statements @@ -32,7 +32,7 @@ export function addImports(importManager: ImportManager, sf: ts.SourceFile, // Prepend imports if needed. if (addedImports.length > 0) { sf.statements = - ts.createNodeArray([...existingImports, ...addedImports, ...extraStatements, ...body]); + ts.createNodeArray([...existingImports, ...addedImports, ...extraStatements, ...body]); } return sf; @@ -40,5 +40,5 @@ export function addImports(importManager: ImportManager, sf: ts.SourceFile, function isImportStatement(stmt: ts.Statement): boolean { return ts.isImportDeclaration(stmt) || ts.isImportEqualsDeclaration(stmt) || - ts.isNamespaceImport(stmt); + ts.isNamespaceImport(stmt); } diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 77305e1595..da5cfa7a5d 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/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'); }); @@ -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'); @@ -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');