From b7a6f52d59c286ece0512fd407d4b9f2ffd9ae1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mis=CC=8Cko=20Hevery?= Date: Wed, 19 Jul 2017 14:01:28 -0700 Subject: [PATCH] perf: latest tsickle to tree shake: abstract class methods & interfaces (#18236) In previous version of tsickle abstract class methods were materialized. The change resulted in 6Kb savings in angular.io bundle. This change also required the removal of `@private` and `@return` type annotation as it is explicitly dissalowed by tsickle. NOTE: removed casts in front of `makeDecorator` due to: https://github.com/angular/devkit/issues/45 ``` 14938 Jul 19 13:16 0.b19e913fbdd6507d346b.chunk.js 1535 Jul 19 13:16 inline.d8e019ea3cfdd86c2bd0.bundle.js 589178 Jul 19 13:16 main.54c97bcb6f254776b678.bundle.js 34333 Jul 19 13:16 polyfills.4a3c9ca9481d53803157.bundle.js 14938 Jul 18 16:55 0.b19e913fbdd6507d346b.chunk.js 1535 Jul 18 16:55 inline.0c83abb44fad9a2768a7.bundle.js 582786 Jul 18 16:55 main.ea290db71b051813e156.bundle.js 34333 Jul 18 16:55 polyfills.4a3c9ca9481d53803157.bundle.js main savings: 589178 - 582786 = 6,392 ``` PR Close #18236 --- .circleci/config.yml | 2 +- npm-shrinkwrap.clean.json | 6 +++--- npm-shrinkwrap.json | 8 ++++---- package.json | 4 ++-- packages/compiler-cli/src/ngtools_api.ts | 4 ---- packages/compiler-cli/src/ngtools_impl.ts | 8 -------- .../compiler/src/expression_parser/lexer.ts | 1 - packages/compiler/src/ml_parser/lexer.ts | 2 +- .../src/template_parser/binding_parser.ts | 1 - packages/compiler/src/url_resolver.ts | 1 - packages/core/src/di/metadata.ts | 2 +- packages/core/src/metadata/di.ts | 11 +++++------ packages/core/src/metadata/directives.ts | 7 +++---- packages/core/src/metadata/ng_module.ts | 2 +- packages/core/src/util/decorators.ts | 5 +++-- .../linker/jit_summaries_integration_spec.ts | 19 +++++++++++-------- packages/core/testing/src/fake_async.ts | 4 ++-- .../platform-browser/src/browser/title.ts | 1 - .../src/security/html_sanitizer.ts | 1 - packages/tsc-wrapped/test/main.spec.ts | 4 ++-- scripts/ci/test-bazel.sh | 2 +- 21 files changed, 40 insertions(+), 55 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 67d9fbe153..04f67a64a4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -42,7 +42,7 @@ jobs: key: angular-{{ .Branch }}-{{ checksum "npm-shrinkwrap.json" }} - run: bazel run @io_bazel_rules_typescript_node//:bin/npm install - - run: bazel build ... + - run: bazel build packages/... - save_cache: key: angular-{{ .Branch }}-{{ checksum "npm-shrinkwrap.json" }} paths: diff --git a/npm-shrinkwrap.clean.json b/npm-shrinkwrap.clean.json index 7eeeb7106c..631cf2fa06 100644 --- a/npm-shrinkwrap.clean.json +++ b/npm-shrinkwrap.clean.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "4.3.0-beta.1", + "version": "4.3.0", "dependencies": { "@types/angularjs": { "version": "1.5.13-alpha" @@ -4818,7 +4818,7 @@ } }, "tsickle": { - "version": "0.21.6" + "version": "0.23.4" }, "tslib": { "version": "1.7.1" @@ -4878,7 +4878,7 @@ "version": "0.0.6" }, "typescript": { - "version": "2.3.2" + "version": "2.3.4" }, "ua-parser-js": { "version": "0.7.10" diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index b3fb372c83..54d14723f9 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "4.3.0-beta.1", + "version": "4.3.0", "dependencies": { "@types/angularjs": { "version": "1.5.13-alpha", @@ -7696,9 +7696,9 @@ } }, "tsickle": { - "version": "0.21.6", - "from": "tsickle@0.21.6", - "resolved": "https://registry.npmjs.org/tsickle/-/tsickle-0.21.6.tgz" + "version": "0.23.4", + "from": "tsickle@0.23.4", + "resolved": "https://registry.npmjs.org/tsickle/-/tsickle-0.23.4.tgz" }, "tslib": { "version": "1.7.1", diff --git a/package.json b/package.json index 0e38db60b7..528e251a34 100644 --- a/package.json +++ b/package.json @@ -92,10 +92,10 @@ "source-map-support": "^0.4.2", "systemjs": "0.18.10", "ts-api-guardian": "^0.2.2", - "tsickle": "^0.21.1", + "tsickle": "^0.23.4", "tslint": "^4.1.1", "tslint-eslint-rules": "^3.1.0", - "typescript": "^2.3.2", + "typescript": "^2.3.4", "universal-analytics": "^0.3.9", "vrsource-tslint-rules": "^4.0.0", "webpack": "^1.12.6", diff --git a/packages/compiler-cli/src/ngtools_api.ts b/packages/compiler-cli/src/ngtools_api.ts index ed88264512..66f7921505 100644 --- a/packages/compiler-cli/src/ngtools_api.ts +++ b/packages/compiler-cli/src/ngtools_api.ts @@ -81,12 +81,10 @@ class CustomLoaderModuleResolutionHostAdapter extends ModuleResolutionHostAdapte /** * @internal - * @private */ export class NgTools_InternalApi_NG_2 { /** * @internal - * @private */ static codeGen(options: NgTools_InternalApi_NG2_CodeGen_Options): Promise { const hostContext: CompilerHostContext = @@ -114,7 +112,6 @@ export class NgTools_InternalApi_NG_2 { /** * @internal - * @private */ static listLazyRoutes(options: NgTools_InternalApi_NG2_ListLazyRoutes_Options): NgTools_InternalApi_NG_2_LazyRouteMap { @@ -144,7 +141,6 @@ export class NgTools_InternalApi_NG_2 { /** * @internal - * @private */ static extractI18n(options: NgTools_InternalApi_NG2_ExtractI18n_Options): Promise { const hostContext: CompilerHostContext = diff --git a/packages/compiler-cli/src/ngtools_impl.ts b/packages/compiler-cli/src/ngtools_impl.ts index fae26b9351..c14f35f09a 100644 --- a/packages/compiler-cli/src/ngtools_impl.ts +++ b/packages/compiler-cli/src/ngtools_impl.ts @@ -49,11 +49,6 @@ export class RouteDef { } -/** - * - * @returns {LazyRouteMap} - * @private - */ export function listLazyRoutesOfModule( entryModule: string, host: AotCompilerHost, reflector: StaticReflector): LazyRouteMap { const entryRouteDef = RouteDef.fromString(entryModule); @@ -111,7 +106,6 @@ function _resolveModule(modulePath: string, containingFile: string, host: AotCom /** * Throw an exception if a route is in a route map, but does not point to the same module. - * @private */ function _assertRoute(map: LazyRouteMap, route: LazyRoute) { const r = route.routeDef.toString(); @@ -173,7 +167,6 @@ function _extractLazyRoutesFromStaticModule( /** * Get the NgModule Metadata of a symbol. - * @private */ function _getNgModuleMetadata(staticSymbol: StaticSymbol, reflector: StaticReflector): NgModule { const ngModules = reflector.annotations(staticSymbol).filter((s: any) => s instanceof NgModule); @@ -204,7 +197,6 @@ function _collectRoutes( /** * Return the loadChildren values of a list of Route. - * @private */ function _collectLoadChildren(routes: Route[]): string[] { return routes.reduce((m, r) => { diff --git a/packages/compiler/src/expression_parser/lexer.ts b/packages/compiler/src/expression_parser/lexer.ts index feae320b7a..17b59c4284 100644 --- a/packages/compiler/src/expression_parser/lexer.ts +++ b/packages/compiler/src/expression_parser/lexer.ts @@ -231,7 +231,6 @@ class _Scanner { * @param two second symbol (part of the operator when the second code point matches) * @param threeCode code point for the third symbol * @param three third symbol (part of the operator when provided and matches source expression) - * @returns {Token} */ scanComplexOperator( start: number, one: string, twoCode: number, two: string, threeCode?: number, diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts index 8257f89929..28adb6e27f 100644 --- a/packages/compiler/src/ml_parser/lexer.ts +++ b/packages/compiler/src/ml_parser/lexer.ts @@ -150,7 +150,7 @@ class _Tokenizer { } /** - * @returns {boolean} whether an ICU token has been created + * @returns whether an ICU token has been created * @internal */ private _tokenizeExpansionForm(): boolean { diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 5066b7757c..b44bcbf8bf 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -401,7 +401,6 @@ export class BindingParser { * @param propName the name of the property / attribute * @param sourceSpan * @param isAttr true when binding to an attribute - * @private */ private _validatePropertyOrAttributeName( propName: string, sourceSpan: ParseSourceSpan, isAttr: boolean): void { diff --git a/packages/compiler/src/url_resolver.ts b/packages/compiler/src/url_resolver.ts index 61dc231e07..b72fb3a2c9 100644 --- a/packages/compiler/src/url_resolver.ts +++ b/packages/compiler/src/url_resolver.ts @@ -202,7 +202,6 @@ function _buildFromEncodedParts( * $6 = query without ? * $7 = Related fragment without # * - * @type {!RegExp} * @internal */ const _splitRe = new RegExp( diff --git a/packages/core/src/di/metadata.ts b/packages/core/src/di/metadata.ts index 91c25540bd..d367785252 100644 --- a/packages/core/src/di/metadata.ts +++ b/packages/core/src/di/metadata.ts @@ -151,7 +151,7 @@ export interface Injectable {} * @stable * @Annotation */ -export const Injectable: InjectableDecorator = makeDecorator('Injectable'); +export const Injectable: InjectableDecorator = makeDecorator('Injectable'); /** * Type of the Self decorator / constructor function. diff --git a/packages/core/src/metadata/di.ts b/packages/core/src/metadata/di.ts index 4b2c5ae784..5efabd0033 100644 --- a/packages/core/src/metadata/di.ts +++ b/packages/core/src/metadata/di.ts @@ -201,12 +201,11 @@ export type ContentChildren = Query; * @stable * @Annotation */ -export const ContentChildren: ContentChildrenDecorator = - makePropDecorator( - 'ContentChildren', - (selector?: any, data: any = {}) => - ({selector, first: false, isViewQuery: false, descendants: false, ...data}), - Query); +export const ContentChildren: ContentChildrenDecorator = makePropDecorator( + 'ContentChildren', + (selector?: any, data: any = {}) => + ({selector, first: false, isViewQuery: false, descendants: false, ...data}), + Query); /** * Type of the ContentChild decorator / constructor function. diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts index 5d4d9b9fe6..501f758e08 100644 --- a/packages/core/src/metadata/directives.ts +++ b/packages/core/src/metadata/directives.ts @@ -400,7 +400,7 @@ export interface Directive { * @Annotation */ export const Directive: DirectiveDecorator = - makeDecorator('Directive', (dir: Directive = {}) => dir); + makeDecorator('Directive', (dir: Directive = {}) => dir); /** * Type of the Component decorator / constructor function. @@ -683,7 +683,7 @@ export interface Component extends Directive { * @stable * @Annotation */ -export const Component: ComponentDecorator = makeDecorator( +export const Component: ComponentDecorator = makeDecorator( 'Component', (c: Component = {}) => ({changeDetection: ChangeDetectionStrategy.Default, ...c}), Directive); @@ -724,8 +724,7 @@ export interface Pipe { * @stable * @Annotation */ -export const Pipe: PipeDecorator = - makeDecorator('Pipe', (p: Pipe) => ({pure: true, ...p})); +export const Pipe: PipeDecorator = makeDecorator('Pipe', (p: Pipe) => ({pure: true, ...p})); /** diff --git a/packages/core/src/metadata/ng_module.ts b/packages/core/src/metadata/ng_module.ts index 6dc53d6c37..c48376605e 100644 --- a/packages/core/src/metadata/ng_module.ts +++ b/packages/core/src/metadata/ng_module.ts @@ -191,4 +191,4 @@ export interface NgModule { * @Annotation */ export const NgModule: NgModuleDecorator = - makeDecorator('NgModule', (ngModule: NgModule) => ngModule); \ No newline at end of file + makeDecorator('NgModule', (ngModule: NgModule) => ngModule); diff --git a/packages/core/src/util/decorators.ts b/packages/core/src/util/decorators.ts index 0ac074677f..1acf99b60b 100644 --- a/packages/core/src/util/decorators.ts +++ b/packages/core/src/util/decorators.ts @@ -263,7 +263,8 @@ export function Class(clsDef: ClassDefinition): Type { */ export function makeDecorator( name: string, props?: (...args: any[]) => any, parentClass?: any, - chainFn?: (fn: Function) => void): (...args: any[]) => (cls: any) => any { + chainFn?: (fn: Function) => void): + {new (...args: any[]): any; (...args: any[]): any; (...args: any[]): (cls: any) => any;} { const metaCtor = makeMetadataCtor(props); function DecoratorFactory(objOrType: any): (cls: any) => any { @@ -298,7 +299,7 @@ export function makeDecorator( DecoratorFactory.prototype.toString = () => `@${name}`; (DecoratorFactory).annotationCls = DecoratorFactory; - return DecoratorFactory; + return DecoratorFactory as any; } function makeMetadataCtor(props?: (...args: any[]) => any): any { diff --git a/packages/core/test/linker/jit_summaries_integration_spec.ts b/packages/core/test/linker/jit_summaries_integration_spec.ts index aaea39659c..149df8d4e0 100644 --- a/packages/core/test/linker/jit_summaries_integration_spec.ts +++ b/packages/core/test/linker/jit_summaries_integration_spec.ts @@ -47,6 +47,14 @@ export function main() { class SomeService extends Base {} + // Move back into the it which needs it after https://github.com/angular/tsickle/issues/547 is + // fixed. + @Component({template: '
{{1 | somePipe}}
'}) + class TestComp3 { + constructor(service: SomeService) {} + } + + function resetTestEnvironmentWithSummaries(summaries?: () => any[]) { const {platform, ngModule} = getTestBed(); TestBed.resetTestEnvironment(); @@ -150,15 +158,10 @@ export function main() { }); it('should use NgModule metadata from summaries', () => { - @Component({template: '
{{1 | somePipe}}
'}) - class TestComp { - constructor(service: SomeService) {} - } - TestBed .configureTestingModule( - {providers: [SomeDep], declarations: [TestComp], imports: [SomeModule]}) - .createComponent(TestComp); + {providers: [SomeDep], declarations: [TestComp3], imports: [SomeModule]}) + .createComponent(TestComp3); expectInstanceCreated(SomeModule); expectInstanceCreated(SomeDirective); @@ -188,4 +191,4 @@ export function main() { .toThrowError('SomeModule was AOT compiled, so its metadata cannot be changed.'); }); }); -} \ No newline at end of file +} diff --git a/packages/core/testing/src/fake_async.ts b/packages/core/testing/src/fake_async.ts index bd5d7f780a..35eb9c8da6 100644 --- a/packages/core/testing/src/fake_async.ts +++ b/packages/core/testing/src/fake_async.ts @@ -43,7 +43,7 @@ let _inFakeAsyncCall = false; * {@example testing/ts/fake_async.ts region='basic'} * * @param fn - * @returns {Function} The function wrapped to be executed in the fakeAsync zone + * @returns The function wrapped to be executed in the fakeAsync zone * * @experimental */ @@ -121,7 +121,7 @@ export function tick(millis: number = 0): void { * of time that would have been elapsed. * * @param maxTurns - * @returns {number} The simulated time elapsed, in millis. + * @returns The simulated time elapsed, in millis. * * @experimental */ diff --git a/packages/platform-browser/src/browser/title.ts b/packages/platform-browser/src/browser/title.ts index 588f323f45..2ca7ba5302 100644 --- a/packages/platform-browser/src/browser/title.ts +++ b/packages/platform-browser/src/browser/title.ts @@ -27,7 +27,6 @@ export class Title { constructor(@Inject(DOCUMENT) private _doc: any) {} /** * Get the title of the current HTML document. - * @returns {string} */ getTitle(): string { return getDOM().getTitle(this._doc); } diff --git a/packages/platform-browser/src/security/html_sanitizer.ts b/packages/platform-browser/src/security/html_sanitizer.ts index c0cccf3530..07fb7c8ade 100644 --- a/packages/platform-browser/src/security/html_sanitizer.ts +++ b/packages/platform-browser/src/security/html_sanitizer.ts @@ -215,7 +215,6 @@ const NON_ALPHANUMERIC_REGEXP = /([^\#-~ |!])/g; * resulting string can be safely inserted into attribute or * element text. * @param value - * @returns {string} escaped text */ function encodeEntities(value: string) { return value.replace(/&/g, '&') diff --git a/packages/tsc-wrapped/test/main.spec.ts b/packages/tsc-wrapped/test/main.spec.ts index 287bd95abc..8cfa7c2ab3 100644 --- a/packages/tsc-wrapped/test/main.spec.ts +++ b/packages/tsc-wrapped/test/main.spec.ts @@ -327,7 +327,7 @@ describe('tsc-wrapped', () => { main(basePath, {basePath}) .then(() => { const out = readOut('js.map'); - expect(out).toContain('"sources":["other_test.ts"]'); + expect(out).toContain('"sources":["other_test.ts","../test.ts"]'); done(); }) .catch(e => done.fail(e)); @@ -361,7 +361,7 @@ describe('tsc-wrapped', () => { main(basePath, {basePath}) .then(() => { const out = readOut('js.map'); - expect(out).toContain('"sources":["other_test.ts"]'); + expect(out).toContain('"sources":["other_test.ts","../test.ts"]'); done(); }) .catch(e => done.fail(e)); diff --git a/scripts/ci/test-bazel.sh b/scripts/ci/test-bazel.sh index 8af994dbe5..9f22e933fe 100755 --- a/scripts/ci/test-bazel.sh +++ b/scripts/ci/test-bazel.sh @@ -2,4 +2,4 @@ set -u -e -o pipefail -bazel build ... \ No newline at end of file +bazel build packages/...