diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 4ed3fb4a85..84a3f75397 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -10,7 +10,6 @@ import {GeneratedFile} from '@angular/compiler'; import * as ts from 'typescript'; import * as api from '../transformers/api'; -import {nocollapseHack} from '../transformers/nocollapse_hack'; import {verifySupportedTypeScriptVersion} from '../typescript_support'; import {NgCompilerHost} from './core'; @@ -193,16 +192,6 @@ export class NgtscProgram implements api.Program { this.compiler.incrementalDriver.recordSuccessfulEmit(writtenSf); } } - - // If Closure annotations are being produced, tsickle should be adding `@nocollapse` to - // any static fields present. However, tsickle doesn't yet handle synthetic fields added - // during other transformations, so this hack is in place to ensure Ivy definitions get - // properly annotated, pending an upstream fix in tsickle. - // - // TODO(alxhub): remove when tsickle properly annotates synthetic fields. - if (this.closureCompilerEnabled && fileName.endsWith('.js')) { - data = nocollapseHack(data); - } this.host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles); }; diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index 23c7198e97..00ada93a58 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -47,7 +47,8 @@ class IvyVisitor extends Visitor { constructor( private compilation: TraitCompiler, private reflector: ReflectionHost, private importManager: ImportManager, private defaultImportRecorder: DefaultImportRecorder, - private isCore: boolean, private constantPool: ConstantPool) { + private isClosureCompilerEnabled: boolean, private isCore: boolean, + private constantPool: ConstantPool) { super(); } @@ -73,6 +74,16 @@ class IvyVisitor extends Visitor { undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], field.name, undefined, undefined, exprNode); + if (this.isClosureCompilerEnabled) { + // Closure compiler transforms the form `Service.ɵprov = X` into `Service$ɵprov = X`. To + // prevent this transformation, such assignments need to be annotated with @nocollapse. + // Note that tsickle is typically responsible for adding such annotations, however it + // doesn't yet handle synthetic fields added during other transformations. + ts.addSyntheticLeadingComment( + property, ts.SyntaxKind.MultiLineCommentTrivia, '* @nocollapse ', + /* hasTrailingNewLine */ false); + } + field.statements .map( stmt => translateStatement( @@ -215,7 +226,8 @@ function transformIvySourceFile( // Recursively scan through the AST and perform any updates requested by the IvyCompilation. const visitor = new IvyVisitor( - compilation, reflector, importManager, defaultImportRecorder, isCore, constantPool); + compilation, reflector, importManager, defaultImportRecorder, isClosureCompilerEnabled, + isCore, constantPool); let sf = visit(file, visitor, context); // Generate the constant statements first, as they may involve adding additional imports diff --git a/packages/compiler-cli/src/transformers/nocollapse_hack.ts b/packages/compiler-cli/src/transformers/nocollapse_hack.ts deleted file mode 100644 index 59cd5de80e..0000000000 --- a/packages/compiler-cli/src/transformers/nocollapse_hack.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * 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 - */ - -// Closure compiler transforms the form `Service.ɵprov = X` into -// `Service$ɵprov = X`. To prevent this transformation, such assignments need to be -// annotated with @nocollapse. Unfortunately, a bug in Typescript where comments aren't propagated -// through the TS transformations precludes adding the comment via the AST. This workaround detects -// the static assignments to R3 properties such as ɵprov using a regex, as output files -// are written, and applies the annotation through regex replacement. -// -// TODO(alxhub): clean up once fix for TS transformers lands in upstream -// -// Typescript reference issue: https://github.com/Microsoft/TypeScript/issues/22497 - -// Pattern matching all Render3 property names. -const R3_DEF_NAME_PATTERN = [ - 'ɵcmp', - 'ɵdir', - 'ɵprov', - 'ɵinj', - 'ɵmod', - 'ɵpipe', - 'ɵfac', -].join('|'); - -// Pattern matching `Identifier.property` where property is a Render3 property. -const R3_DEF_ACCESS_PATTERN = `[^\\s\\.()[\\]]+\.(${R3_DEF_NAME_PATTERN})`; - -// Pattern matching a source line that contains a Render3 static property assignment. -// It declares two matching groups - one for the preceding whitespace, the second for the rest -// of the assignment expression. -const R3_DEF_LINE_PATTERN = `^(\\s*)(${R3_DEF_ACCESS_PATTERN} = .*)$`; - -// Regex compilation of R3_DEF_LINE_PATTERN. Matching group 1 yields the whitespace preceding the -// assignment, matching group 2 gives the rest of the assignment expressions. -const R3_MATCH_DEFS = new RegExp(R3_DEF_LINE_PATTERN, 'gmu'); - -const R3_TSICKLE_DECL_PATTERN = - `(\\/\\*\\*[*\\s]*)(@[^*]+\\*\\/\\s+[^.]+\\.(?:${R3_DEF_NAME_PATTERN});)`; - -const R3_MATCH_TSICKLE_DECL = new RegExp(R3_TSICKLE_DECL_PATTERN, 'gmu'); - -// Replacement string that complements R3_MATCH_DEFS. It inserts `/** @nocollapse */` before the -// assignment but after any indentation. Note that this will mess up any sourcemaps on this line -// (though there shouldn't be any, since Render3 properties are synthetic). -const R3_NOCOLLAPSE_DEFS = '$1\/** @nocollapse *\/ $2'; - -const R3_NOCOLLAPSE_TSICKLE_DECL = '$1@nocollapse $2'; - -export function nocollapseHack(contents: string): string { - return contents.replace(R3_MATCH_DEFS, R3_NOCOLLAPSE_DEFS) - .replace(R3_MATCH_TSICKLE_DECL, R3_NOCOLLAPSE_TSICKLE_DECL); -} diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index 1cd37b45e8..9171982655 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -20,9 +20,11 @@ const CATCH_STACK_NAME = 'stack'; const _VALID_IDENTIFIER_RE = /^[$A-Z_][0-9A-Z_$]*$/i; export class TypeScriptNodeEmitter { + constructor(private annotateForClosureCompiler: boolean) {} + updateSourceFile(sourceFile: ts.SourceFile, stmts: Statement[], preamble?: string): [ts.SourceFile, Map] { - const converter = new NodeEmitterVisitor(); + const converter = new NodeEmitterVisitor(this.annotateForClosureCompiler); // [].concat flattens the result so that each `visit...` method can also return an array of // stmts. const statements: any[] = [].concat( @@ -63,8 +65,8 @@ export class TypeScriptNodeEmitter { */ export function updateSourceFile( sourceFile: ts.SourceFile, module: PartialModule, - context: ts.TransformationContext): [ts.SourceFile, Map] { - const converter = new NodeEmitterVisitor(); + annotateForClosureCompiler: boolean): [ts.SourceFile, Map] { + const converter = new NodeEmitterVisitor(annotateForClosureCompiler); converter.loadExportedVariableIdentifiers(sourceFile); const prefixStatements = module.statements.filter(statement => !(statement instanceof ClassStmt)); @@ -192,6 +194,8 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { private _templateSources = new Map(); private _exportedVariableIdentifiers = new Map(); + constructor(private annotateForClosureCompiler: boolean) {} + /** * Process the source file and collect exported identifiers that refer to variables. * @@ -367,14 +371,27 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { visitDeclareClassStmt(stmt: ClassStmt) { const modifiers = this.getModifiers(stmt); - const fields = stmt.fields.map( - field => ts.createProperty( - /* decorators */ undefined, /* modifiers */ translateModifiers(field.modifiers), - field.name, - /* questionToken */ undefined, - /* type */ undefined, - field.initializer == null ? ts.createNull() : - field.initializer.visitExpression(this, null))); + const fields = stmt.fields.map(field => { + const property = ts.createProperty( + /* decorators */ undefined, /* modifiers */ translateModifiers(field.modifiers), + field.name, + /* questionToken */ undefined, + /* type */ undefined, + field.initializer == null ? ts.createNull() : + field.initializer.visitExpression(this, null)); + + if (this.annotateForClosureCompiler) { + // Closure compiler transforms the form `Service.ɵprov = X` into `Service$ɵprov = X`. To + // prevent this transformation, such assignments need to be annotated with @nocollapse. + // Note that tsickle is typically responsible for adding such annotations, however it + // doesn't yet handle synthetic fields added during other transformations. + ts.addSyntheticLeadingComment( + property, ts.SyntaxKind.MultiLineCommentTrivia, '* @nocollapse ', + /* hasTrailingNewLine */ false); + } + + return property; + }); const getters = stmt.getters.map( getter => ts.createGetAccessor( /* decorators */ undefined, /* modifiers */ undefined, getter.name, /* parameters */[], diff --git a/packages/compiler-cli/src/transformers/node_emitter_transform.ts b/packages/compiler-cli/src/transformers/node_emitter_transform.ts index fffdc8e13e..bd72bd06e2 100644 --- a/packages/compiler-cli/src/transformers/node_emitter_transform.ts +++ b/packages/compiler-cli/src/transformers/node_emitter_transform.ts @@ -30,10 +30,10 @@ function getPreamble(original: string) { * list of statements as their body. */ export function getAngularEmitterTransformFactory( - generatedFiles: Map, program: ts.Program): () => - (sourceFile: ts.SourceFile) => ts.SourceFile { + generatedFiles: Map, program: ts.Program, + annotateForClosureCompiler: boolean): () => (sourceFile: ts.SourceFile) => ts.SourceFile { return function() { - const emitter = new TypeScriptNodeEmitter(); + const emitter = new TypeScriptNodeEmitter(annotateForClosureCompiler); return function(sourceFile: ts.SourceFile): ts.SourceFile { const g = generatedFiles.get(sourceFile.fileName); const orig = g && program.getSourceFile(g.srcFileUrl); diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index 3855bddf1b..eb631c4c2d 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -22,7 +22,6 @@ import {CodeGenerator, TsCompilerAotCompilerTypeCheckHostAdapter, getOriginalRef import {InlineResourcesMetadataTransformer, getInlineResourcesTransformFactory} from './inline_resources'; import {LowerMetadataTransform, getExpressionLoweringTransformFactory} from './lower_expressions'; import {MetadataCache, MetadataTransformer} from './metadata_cache'; -import {nocollapseHack} from './nocollapse_hack'; import {getAngularEmitterTransformFactory} from './node_emitter_transform'; import {PartialModuleMetadataTransformer} from './r3_metadata_transform'; import {StripDecoratorsMetadataTransformer, getDecoratorStripTransformerFactory} from './r3_strip_decorators'; @@ -281,12 +280,6 @@ class AngularCompilerProgram implements Program { const writeTsFile: ts.WriteFileCallback = (outFileName, outData, writeByteOrderMark, onError?, sourceFiles?) => { - const sourceFile = sourceFiles && sourceFiles.length == 1 ? sourceFiles[0] : null; - let genFile: GeneratedFile|undefined; - if (this.options.annotateForClosureCompiler && sourceFile && - TS.test(sourceFile.fileName)) { - outData = nocollapseHack(outData); - } this.writeFile(outFileName, outData, writeByteOrderMark, onError, undefined, sourceFiles); }; @@ -377,9 +370,6 @@ class AngularCompilerProgram implements Program { emittedSourceFiles.push(originalFile); } } - if (this.options.annotateForClosureCompiler && TS.test(sourceFile.fileName)) { - outData = nocollapseHack(outData); - } } this.writeFile(outFileName, outData, writeByteOrderMark, onError, genFile, sourceFiles); }; @@ -572,11 +562,13 @@ class AngularCompilerProgram implements Program { getExpressionLoweringTransformFactory(this.loweringMetadataTransform, this.tsProgram)); metadataTransforms.push(this.loweringMetadataTransform); } + const annotateForClosureCompiler = this.options.annotateForClosureCompiler || false; if (genFiles) { - beforeTs.push(getAngularEmitterTransformFactory(genFiles, this.getTsProgram())); + beforeTs.push(getAngularEmitterTransformFactory( + genFiles, this.getTsProgram(), annotateForClosureCompiler)); } if (partialModules) { - beforeTs.push(getAngularClassTransformerFactory(partialModules)); + beforeTs.push(getAngularClassTransformerFactory(partialModules, annotateForClosureCompiler)); // If we have partial modules, the cached metadata might be incorrect as it doesn't reflect // the partial module transforms. diff --git a/packages/compiler-cli/src/transformers/r3_transform.ts b/packages/compiler-cli/src/transformers/r3_transform.ts index 5c21b8bc82..cf61b8ad2f 100644 --- a/packages/compiler-cli/src/transformers/r3_transform.ts +++ b/packages/compiler-cli/src/transformers/r3_transform.ts @@ -17,7 +17,8 @@ export type TransformerFactory = (context: ts.TransformationContext) => Transfor /** * Returns a transformer that adds the requested static methods specified by modules. */ -export function getAngularClassTransformerFactory(modules: PartialModule[]): TransformerFactory { +export function getAngularClassTransformerFactory( + modules: PartialModule[], annotateForClosureCompiler: boolean): TransformerFactory { if (modules.length === 0) { // If no modules are specified, just return an identity transform. return () => sf => sf; @@ -27,7 +28,7 @@ export function getAngularClassTransformerFactory(modules: PartialModule[]): Tra return function(sourceFile: ts.SourceFile): ts.SourceFile { const module = moduleMap.get(sourceFile.fileName); if (module && module.statements.length > 0) { - const [newSourceFile] = updateSourceFile(sourceFile, module, context); + const [newSourceFile] = updateSourceFile(sourceFile, module, annotateForClosureCompiler); return newSourceFile; } return sourceFile; diff --git a/packages/compiler-cli/test/transformers/nocollapse_hack_spec.ts b/packages/compiler-cli/test/transformers/nocollapse_hack_spec.ts deleted file mode 100644 index 38742da439..0000000000 --- a/packages/compiler-cli/test/transformers/nocollapse_hack_spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * 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 {nocollapseHack} from '../../src/transformers/nocollapse_hack'; - -describe('@nocollapse hack', () => { - it('should add @nocollapse to a basic class', () => { - const decl = `Foo.ɵinj = define(...);`; - expect(nocollapseHack(decl)).toEqual('/** @nocollapse */ ' + decl); - }); - - it('should add nocollapse to an if (false) declaration of the kind generated by tsickle', () => { - const decl = ` - if (false) { - /** @type {?} */ - Foo.ɵinj; - } - `; - expect(nocollapseHack(decl)).toContain('/** @nocollapse @type {?} */'); - }); -}); diff --git a/packages/compiler-cli/test/transformers/node_emitter_spec.ts b/packages/compiler-cli/test/transformers/node_emitter_spec.ts index 9fdbe6ccbb..c5e3ce09cb 100644 --- a/packages/compiler-cli/test/transformers/node_emitter_spec.ts +++ b/packages/compiler-cli/test/transformers/node_emitter_spec.ts @@ -33,7 +33,7 @@ describe('TypeScriptNodeEmitter', () => { beforeEach(() => { context = new MockAotContext('/', FILES); host = new MockCompilerHost(context); - emitter = new TypeScriptNodeEmitter(); + emitter = new TypeScriptNodeEmitter(false); someVar = o.variable('someVar', null, null); }); diff --git a/packages/compiler-cli/test/transformers/r3_transform_spec.ts b/packages/compiler-cli/test/transformers/r3_transform_spec.ts index 992165f086..5b893cf6de 100644 --- a/packages/compiler-cli/test/transformers/r3_transform_spec.ts +++ b/packages/compiler-cli/test/transformers/r3_transform_spec.ts @@ -40,14 +40,16 @@ describe('r3_transform_spec', () => { }); it('should be able to modify multiple classes in the same module', () => { - const result = emit(getAngularClassTransformerFactory([{ - fileName: someGenFileName, - statements: [ - classMethod(new o.ReturnStatement(o.variable('v')), ['v'], 'someMethod', 'SomeClass'), - classMethod( - new o.ReturnStatement(o.variable('v')), ['v'], 'someOtherMethod', 'SomeOtherClass') - ] - }])); + const result = emit(getAngularClassTransformerFactory( + [{ + fileName: someGenFileName, + statements: [ + classMethod(new o.ReturnStatement(o.variable('v')), ['v'], 'someMethod', 'SomeClass'), + classMethod( + new o.ReturnStatement(o.variable('v')), ['v'], 'someOtherMethod', 'SomeOtherClass') + ] + }], + false)); expect(result).toContain('static someMethod(v) { return v; }'); expect(result).toContain('static someOtherMethod(v) { return v; }'); }); @@ -94,7 +96,7 @@ describe('r3_transform_spec', () => { fileName: someGenFileName, statements: [classMethod(stmt, parameters, methodName, className)] }; - return emit(getAngularClassTransformerFactory([module])); + return emit(getAngularClassTransformerFactory([module], false)); } function emitStaticField( @@ -104,7 +106,7 @@ describe('r3_transform_spec', () => { fileName: someGenFileName, statements: [classField(initializer, fieldName, className)] }; - return emit(getAngularClassTransformerFactory([module])); + return emit(getAngularClassTransformerFactory([module], false)); } });