Skip to content

Commit

Permalink
refactor(compiler-cli): properly preserve file overview comments (#54983
Browse files Browse the repository at this point in the history
)

This commit updates the logic for preserving file overview comments
to be more reliable and less dependent on previous transforms.

Previously, with the old import manager, we had a utility called
`addImport` that always separated import statements and non-import
statements. This meant that the non-emitted statement from Tsickle
for the synthetic file-overview comments no longer lived at the
beginning of the file.

`addImports` tried to overcome this by adding another new non-emitted
statement *before* all imports. This then was later used by the
transform (or was assumed!) to attach the synthetic file overview
comments if the original tsickle AST Node is no longer at the top.

This logic can be improved, because the import manager shouldn't need to
bother about this fileoverview non-emitted statement, and the logic for
re-attaching the fileoverview comment should be local. This commit fixes
this and makes it a local transform.

PR Close #54983
  • Loading branch information
devversion authored and dylhunn committed Mar 27, 2024
1 parent ed271eb commit 93ce4d0
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 4 deletions.
16 changes: 13 additions & 3 deletions packages/compiler-cli/src/ngtsc/transform/src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ function transformIvySourceFile(
sf = importManager.transformTsFile(context, sf, constants);

if (fileOverviewMeta !== null) {
setFileOverviewComment(sf, fileOverviewMeta);
sf = insertFileOverviewComment(sf, fileOverviewMeta);
}

return sf;
Expand Down Expand Up @@ -382,7 +382,8 @@ function getFileOverviewComment(statements: ts.NodeArray<ts.Statement>): FileOve
return null;
}

function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMeta): void {
function insertFileOverviewComment(
sf: ts.SourceFile, fileoverview: FileOverviewMeta): ts.SourceFile {
const {comments, host, trailing} = fileoverview;
// If host statement is no longer the first one, it means that extra statements were added at the
// very beginning, so we need to relocate @fileoverview comment and cleanup the original statement
Expand All @@ -393,8 +394,17 @@ function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMet
} else {
ts.setSyntheticLeadingComments(host, undefined);
}
ts.setSyntheticLeadingComments(sf.statements[0], comments);

// Note: Do not use the first statement as it may be elided at runtime.
// E.g. an import declaration that is type only.
const commentNode = ts.factory.createNotEmittedStatement(sf);
ts.setSyntheticLeadingComments(commentNode, comments);

return ts.factory.updateSourceFile(
sf, [commentNode, ...sf.statements], sf.isDeclarationFile, sf.referencedFiles,
sf.typeReferenceDirectives, sf.hasNoDefaultLib, sf.libReferenceDirectives);
}
return sf;
}

function maybeFilterDecorator(
Expand Down
59 changes: 58 additions & 1 deletion packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {NgtscProgram} from '@angular/compiler-cli/src/ngtsc/program';
import {CompilerOptions} from '@angular/compiler-cli/src/transformers/api';
import {createCompilerHost} from '@angular/compiler-cli/src/transformers/compiler_host';
import {platform} from 'os';
import ts from 'typescript';

import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
import {absoluteFrom} from '../../src/ngtsc/file_system';
import {absoluteFrom, NgtscCompilerHost} from '../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {loadStandardTestFiles} from '../../src/ngtsc/testing';
import {restoreTypeScriptVersionForTesting, setTypeScriptVersionForTesting} from '../../src/typescript_support';
Expand Down Expand Up @@ -9294,6 +9297,60 @@ function allTests(os: string) {
.toContain(
'{ i0.ɵɵtwoWayBindingSet(ctx.a, $event) || (ctx.a = $event); return $event; }');
});

describe('tsickle compatibility', () => {
it('should preserve fileoverview comments', () => {
env.write('test.ts', `
// type-only import that will be elided.
import {SimpleChanges} from '@angular/core';
export class X {
p: SimpleChanges = null!;
}
`);

const options: CompilerOptions = {
strict: true,
strictTemplates: true,
target: ts.ScriptTarget.Latest,
module: ts.ModuleKind.ESNext,
annotateForClosureCompiler: true,
};

const program = new NgtscProgram(['/test.ts'], options, createCompilerHost({options}));
const transformers = program.compiler.prepareEmit().transformers;

// Add a "fake tsickle" transform before Angular's transform.
transformers.before!.unshift(ctx => (sf: ts.SourceFile) => {
const tsickleFileOverview = ctx.factory.createNotEmittedStatement(sf);
ts.setSyntheticLeadingComments(tsickleFileOverview, [
{
kind: ts.SyntaxKind.MultiLineCommentTrivia,
text: `*
* @fileoverview Closure comment
* @supress bla1
* @supress bla2
`,
pos: -1,
end: -1,
hasTrailingNewLine: true,
},
]);
return ctx.factory.updateSourceFile(
sf, [tsickleFileOverview, ...sf.statements], sf.isDeclarationFile,
sf.referencedFiles, sf.typeReferenceDirectives, sf.hasNoDefaultLib,
sf.libReferenceDirectives);
});

const {diagnostics, emitSkipped} =
program.getTsProgram().emit(undefined, undefined, undefined, undefined, transformers);

expect(diagnostics.length).toBe(0);
expect(emitSkipped).toBe(false);

expect(env.getContents('/test.js')).toContain(`* @fileoverview Closure comment`);
});
});
});
});

Expand Down

0 comments on commit 93ce4d0

Please sign in to comment.