From 8c682c52b112b440319b2a88e657d6eda47f3992 Mon Sep 17 00:00:00 2001 From: David Neil Date: Thu, 21 May 2020 21:26:43 -0600 Subject: [PATCH] fix(ngcc): use annotateForClosureCompiler option (#36652) Adds @nocollapse to static properties added by ngcc iff annotateForClosureCompiler is true. The Closure Compiler will collapse static properties into the global namespace. Adding this annotation keeps the properties attached to their respective object, which allows them to be referenced via a class's constructor. The annotation is already added by ngtsc and ngc under the same option, this commit extends the functionality to ngcc. Closes #36618. PR Close #36652 --- .../ngcc/src/analysis/decoration_analyzer.ts | 6 ++-- .../ngcc/src/packages/transformer.ts | 3 +- .../ngcc/src/rendering/renderer.ts | 35 +++++++++++++------ .../ngcc/test/integration/ngcc_spec.ts | 16 +++++++++ .../src/ngtsc/translator/src/translator.ts | 10 ++++-- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 172263a142..39c33405f8 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -99,13 +99,13 @@ export class DecorationAnalyzer { /* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat, /* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, NOOP_DEPENDENCY_TRACKER, - this.injectableRegistry, /* annotateForClosureCompiler */ false), + this.injectableRegistry, !!this.compilerOptions.annotateForClosureCompiler), // See the note in ngtsc about why this cast is needed. // clang-format off new DirectiveDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry, this.fullMetaReader, NOOP_DEFAULT_IMPORT_RECORDER, this.injectableRegistry, this.isCore, - /* annotateForClosureCompiler */ false, + !!this.compilerOptions.annotateForClosureCompiler, // In ngcc we want to compile undecorated classes with Angular features. As of // version 10, undecorated classes that use Angular features are no longer handled // in ngtsc, but we want to ensure compatibility in ngcc for outdated libraries that @@ -126,7 +126,7 @@ export class DecorationAnalyzer { this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null, this.refEmitter, /* factoryTracker */ null, NOOP_DEFAULT_IMPORT_RECORDER, - /* annotateForClosureCompiler */ false, this.injectableRegistry), + !!this.compilerOptions.annotateForClosureCompiler, this.injectableRegistry), ]; compiler = new NgccTraitCompiler(this.handlers, this.reflectionHost); migrations: Migration[] = [ diff --git a/packages/compiler-cli/ngcc/src/packages/transformer.ts b/packages/compiler-cli/ngcc/src/packages/transformer.ts index 05f19fd0b0..baa4cf7b84 100644 --- a/packages/compiler-cli/ngcc/src/packages/transformer.ts +++ b/packages/compiler-cli/ngcc/src/packages/transformer.ts @@ -94,7 +94,8 @@ export class Transformer { // Transform the source files and source maps. const srcFormatter = this.getRenderingFormatter(ngccReflectionHost, bundle); - const renderer = new Renderer(reflectionHost, srcFormatter, this.fs, this.logger, bundle); + const renderer = + new Renderer(reflectionHost, srcFormatter, this.fs, this.logger, bundle, this.tsConfig); let renderedFiles = renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index f0ab830a2c..a5dc34c89e 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -5,12 +5,13 @@ * 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 {ConstantPool, Expression, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler'; +import {CommentStmt, ConstantPool, Expression, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler'; import MagicString from 'magic-string'; import * as ts from 'typescript'; import {FileSystem} from '../../../src/ngtsc/file_system'; import {ImportManager} from '../../../src/ngtsc/translator'; +import {ParsedConfiguration} from '../../../src/perform_compile'; import {PrivateDeclarationsAnalyses} from '../analysis/private_declarations_analyzer'; import {SwitchMarkerAnalyses, SwitchMarkerAnalysis} from '../analysis/switch_marker_analyzer'; import {CompiledClass, CompiledFile, DecorationAnalyses} from '../analysis/types'; @@ -32,7 +33,8 @@ import {FileToWrite, getImportRewriter, stripExtension} from './utils'; export class Renderer { constructor( private host: NgccReflectionHost, private srcFormatter: RenderingFormatter, - private fs: FileSystem, private logger: Logger, private bundle: EntryPointBundle) {} + private fs: FileSystem, private logger: Logger, private bundle: EntryPointBundle, + private tsConfig: ParsedConfiguration|null = null) {} renderProgram( decorationAnalyses: DecorationAnalyses, switchMarkerAnalyses: SwitchMarkerAnalyses, @@ -82,8 +84,9 @@ export class Renderer { this.srcFormatter.removeDecorators(outputText, decoratorsToRemove); compiledFile.compiledClasses.forEach(clazz => { - const renderedDefinition = - this.renderDefinitions(compiledFile.sourceFile, clazz, importManager); + const renderedDefinition = this.renderDefinitions( + compiledFile.sourceFile, clazz, importManager, + !!this.tsConfig?.options.annotateForClosureCompiler); this.srcFormatter.addDefinitions(outputText, clazz, renderedDefinition); const renderedStatements = @@ -160,12 +163,14 @@ export class Renderer { * @param imports An object that tracks the imports that are needed by the rendered definitions. */ private renderDefinitions( - sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string { + sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager, + annotateForClosureCompiler: boolean): string { const name = this.host.getInternalNameOfClass(compiledClass.declaration); - const statements: Statement[] = compiledClass.compilation.map(c => { - return createAssignmentStatement(name, c.name, c.initializer); + const statements: Statement[][] = compiledClass.compilation.map(c => { + return createAssignmentStatements( + name, c.name, c.initializer, annotateForClosureCompiler ? '* @nocollapse ' : undefined); }); - return this.renderStatements(sourceFile, statements, imports); + return this.renderStatements(sourceFile, Array.prototype.concat.apply([], statements), imports); } /** @@ -208,8 +213,16 @@ export function renderConstantPool( * compiled decorator to be applied to the class. * @param analyzedClass The info about the class whose statement we want to create. */ -function createAssignmentStatement( - receiverName: ts.DeclarationName, propName: string, initializer: Expression): Statement { +function createAssignmentStatements( + receiverName: ts.DeclarationName, propName: string, initializer: Expression, + leadingComment?: string): Statement[] { const receiver = new WrappedNodeExpr(receiverName); - return new WritePropExpr(receiver, propName, initializer).toStmt(); + const statements = + [new WritePropExpr( + receiver, propName, initializer, /* type */ undefined, /* sourceSpan */ undefined) + .toStmt()]; + if (leadingComment !== undefined) { + statements.unshift(new CommentStmt(leadingComment, true)); + } + return statements; } diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index dbf6dd65c0..eb9da5d677 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -1541,6 +1541,22 @@ runInEachFileSystem(() => { }); }); + describe('with Closure Compiler', () => { + it('should give closure annotated output with annotateForClosureCompiler: true', () => { + fs.writeFile( + _('/tsconfig.json'), + JSON.stringify({angularCompilerOptions: {annotateForClosureCompiler: true}})); + mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']}); + const jsContents = fs.readFile(_(`/dist/local-package/index.js`)); + expect(jsContents).toContain('/** @nocollapse */ \nAppComponent.ɵcmp ='); + }); + it('should default to not give closure annotated output', () => { + mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']}); + const jsContents = fs.readFile(_(`/dist/local-package/index.js`)); + expect(jsContents).not.toContain('/** @nocollapse */'); + }); + }); + describe('with configuration files', () => { it('should process a configured deep-import as an entry-point', () => { loadTestFiles([ diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 1527b8f17a..699f8e1478 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -187,8 +187,14 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor return ts.createThrow(stmt.error.visitExpression(this, context.withExpressionMode)); } - visitCommentStmt(stmt: CommentStmt, context: Context): never { - throw new Error('Method not implemented.'); + visitCommentStmt(stmt: CommentStmt, context: Context): ts.NotEmittedStatement { + const commentStmt = ts.createNotEmittedStatement(ts.createLiteral('')); + ts.addSyntheticLeadingComment( + commentStmt, + stmt.multiline ? ts.SyntaxKind.MultiLineCommentTrivia : + ts.SyntaxKind.SingleLineCommentTrivia, + stmt.comment, /** hasTrailingNewLine */ false); + return commentStmt; } visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: Context): ts.NotEmittedStatement {