iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(bazel): transform generated shims (in Ivy) with tsickle (#35975) 

Currently, when Angular code is built with Bazel and with Ivy, generated
factory shims (.ngfactory files) are not processed via the majority of
tsickle's transforms. This is a subtle effect of the build infrastructure,
but it boils down to a TsickleHost method `shouldSkipTsickleProcessing`.

For ngc_wrapped builds (Bazel + Angular), this method is defined in the
`@bazel/typescript` (aka bazel rules_typescript) implementation of
`CompilerHost`. The default behavior is to skip tsickle processing for files
which are not present in the original `srcs[]` of the build rule. In
Angular's case, this includes all generated shim files.

For View Engine factories this is probably desirable as they're quite
complex and they've never been tested with tsickle. Ivy factories however
are smaller and very straightforward, and it makes sense to treat them like
any other output.

This commit adjusts two independent implementations of
`shouldSkipTsickleProcessing` to enable transformation of Ivy shims:

* in `@angular/bazel` aka ngc_wrapped, the upstream `@bazel/typescript`
  `CompilerHost` is patched to treat .ngfactory files the same as their
  original source file, with respect to tsickle processing.

  It is currently not possible to test this change as we don't have any test
  that inspects tsickle output with bazel. It will be extensively tested in
  g3.

* in `ngc`, Angular's own implementation is adjusted to allow for the
  processing of shims when compiling with Ivy. This enables a unit test to
  be written to validate the correct behavior of tsickle when given a host
  that's appropriately configured to process factory shims.

For ngtsc-as-a-plugin, a similar fix will need to be submitted upstream in
tsc_wrapped.

PR Close #35848

PR Close #35975
This commit is contained in:
Alex Rickabaugh 2020-03-03 17:02:39 -08:00 committed by Andrew Kushnir
parent 4af2a068c5
commit e3ecdc6a63
4 changed files with 120 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
packages/bazel/src/ngc-wrapped/index.ts
View File

@ -184,7 +184,7 @@ export function compile({allDepsCompiledWithBazel = true, useManifestPathsAsModu
}
// Detect from compilerOpts whether the entrypoint is being invoked in Ivy mode.
const isInIvyMode = compilerOpts.enableIvy === 'ngtsc';
const isInIvyMode = !!compilerOpts.enableIvy;
// Disable downleveling and Closure annotation if in Ivy mode.
if (isInIvyMode) {
@ -246,9 +246,18 @@ export function compile({allDepsCompiledWithBazel = true, useManifestPathsAsModu
files, compilerOpts, bazelOpts, tsHost, fileLoader, generatedFileModuleResolver);
}
// Also need to disable decorator downleveling in the BazelHost in Ivy mode.
if (isInIvyMode) {
// Also need to disable decorator downleveling in the BazelHost in Ivy mode.
bazelHost.transformDecorators = false;
const delegate = bazelHost.shouldSkipTsickleProcessing.bind(bazelHost);
bazelHost.shouldSkipTsickleProcessing = (fileName: string) => {
// The base implementation of shouldSkipTsickleProcessing checks whether `fileName` is part of
// the original `srcs[]`. For Angular (Ivy) compilations, ngfactory/ngsummary files that are
// shims for original .ts files in the program should be treated identically. Thus, strip the
// '.ngfactory' or '.ngsummary' part of the filename away before calling the delegate.
return delegate(fileName.replace(/\.(ngfactory|ngsummary)\.ts$/, '.ts'));
};
}
// Prevent tsickle adding any types at all if we don't want closure compiler annotations.

5
packages/compiler-cli/src/main.ts
View File

@ -103,8 +103,9 @@ function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|un
tsickle.TsickleHost, 'shouldSkipTsickleProcessing'|'pathToModuleName'|
'shouldIgnoreWarningsForPath'|'fileNameToModuleId'|'googmodule'|'untyped'|
'convertIndexImportShorthand'|'transformDecorators'|'transformTypesToClosure'> = {
shouldSkipTsickleProcessing: (fileName) =>
/\.d\.ts$/.test(fileName) || GENERATED_FILES.test(fileName),
shouldSkipTsickleProcessing: (fileName) => /\.d\.ts$/.test(fileName) ||
// View Engine's generated files were never intended to be processed with tsickle.
(!options.enableIvy && GENERATED_FILES.test(fileName)),
pathToModuleName: (context, importPath) => '',
shouldIgnoreWarningsForPath: (filePath) => false,
fileNameToModuleId: (fileName) => fileName,

50
packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts
View File

@ -56,21 +56,16 @@ export class FactoryGenerator implements ShimGenerator {
.map(decl => decl.name !.text);
let sourceText = '';
// If there is a top-level comment in the original file, copy it over at the top of the
// generated factory file. This is important for preserving any load-bearing jsdoc comments.
let comment: string = '';
if (original.statements.length > 0) {
const firstStatement = original.statements[0];
// Must pass SourceFile to getLeadingTriviaWidth() and getFullText(), otherwise it'll try to
// get SourceFile by recursively looking up the parent of the Node and fail,
// because parent is undefined.
const leadingTriviaWidth = firstStatement.getLeadingTriviaWidth(original);
if (leadingTriviaWidth > 0) {
comment = firstStatement.getFullText(original).substr(0, leadingTriviaWidth);
}
const leadingComment = getFileoverviewComment(original);
if (leadingComment !== null) {
// Leading comments must be separated from the rest of the contents by a blank line.
sourceText = leadingComment + '\n\n';
}
let sourceText = comment;
if (symbolNames.length > 0) {
// For each symbol name, generate a constant export of the corresponding NgFactory.
// This will encompass a lot of symbols which don't need factories, but that's okay
@ -248,3 +243,36 @@ function transformFactorySourceFile(
return file;
}
/**
* Parses and returns the comment text of a \@fileoverview comment in the given source file.
*/
function getFileoverviewComment(sourceFile: ts.SourceFile): string|null {
const text = sourceFile.getFullText();
const trivia = text.substring(0, sourceFile.getStart());
const leadingComments = ts.getLeadingCommentRanges(trivia, 0);
if (!leadingComments || leadingComments.length === 0) {
return null;
}
const comment = leadingComments[0];
if (comment.kind !== ts.SyntaxKind.MultiLineCommentTrivia) {
return null;
}
// Only comments separated with a \n\n from the file contents are considered file-level comments
// in TypeScript.
if (text.substring(comment.end, comment.end + 2) !== '\n\n') {
return null;
}
const commentText = text.substring(comment.pos, comment.end);
// Closure Compiler ignores @suppress and similar if the comment contains @license.
if (commentText.indexOf('@license') !== -1) {
return null;
}
return commentText;
}

82
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -3322,21 +3322,6 @@ runInEachFileSystem(os => {
expect(ngfactoryContents).toContain(`i0.ɵNgModuleFactory<any>`);
});
it('should copy a top-level comment into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `/** I am a top-level comment. */
import {NgModule} from '@angular/core';
@NgModule({})
export class TestModule {}
`);
env.driveMain();
const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).toMatch(/^\/\*\* I am a top-level comment\. \*\//);
});
it('should be able to compile an app using the factory shim', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
@ -3376,6 +3361,48 @@ runInEachFileSystem(os => {
export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
`));
});
describe('file-level comments', () => {
it('should copy a top-level comment into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `/** I am a top-level comment. */
import {NgModule} from '@angular/core';
@NgModule({})
export class TestModule {}
`);
env.driveMain();
const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).toContain(`/** I am a top-level comment. */\n`);
});
it('should not copy a non-file level comment into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `/** I am a top-level comment, but not for the file. */
export const TEST = true;
`);
env.driveMain();
const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).not.toContain('top-level comment');
});
it('should not copy a file level comment with an @license into a factory stub', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `/** @license I am a top-level comment, but have a license. */
export const TEST = true;
`);
env.driveMain();
const factoryContents = env.getContents('test.ngfactory.js');
expect(factoryContents).not.toContain('top-level comment');
});
});
});
@ -4801,6 +4828,31 @@ runInEachFileSystem(os => {
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});
it('should be produced for generated factory files', () => {
env.tsconfig({
'annotateForClosureCompiler': true,
'generateNgFactoryShims': true,
});
env.write(`test.ts`, `
import {Component} from '@angular/core';
@Component({
template: '<div class="test"></div>',
})
export class SomeComp {}
`);
env.driveMain();
const jsContents = env.getContents('test.ngfactory.js');
const fileoverview = `
/**
* @fileoverview added by tsickle
* @suppress {checkTypes,constantProperty,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
*/
`;
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});
it('should always be at the very beginning of a script (if placed above imports)', () => {
env.tsconfig({
'annotateForClosureCompiler': true,