From 264950bbf2199386d77d2fe7fa7684d003afc1ff Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Thu, 23 Jul 2020 18:51:55 -0700 Subject: [PATCH] fix(compiler): share identical stylesheets between components in the same file (#38213) Prior to this commit, duplicated styles defined in multiple components in the same file were not shared between components, thus causing extra payload size. This commit updates compiler logic to use `ConstantPool` for the styles (while generating the `styles` array on component def), which enables styles sharing when needed (when duplicates styles are present). Resolves #38204. PR Close #38213 --- goldens/size-tracking/aio-payloads.json | 2 +- .../src/ngtsc/transform/src/transform.ts | 8 ++- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 62 +++++++++++++++++++ packages/compiler/src/constant_pool.ts | 2 +- .../compiler/src/render3/view/compiler.ts | 2 +- 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index 2790f7a29f8..7cf693ed9f1 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 450301, + "main-es2015": 448184, "polyfills-es2015": 52630 } } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index c587ebcc152..99933680d06 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -86,13 +86,15 @@ class IvyTransformationVisitor extends Visitor { VisitListEntryResult { // If this class is not registered in the map, it means that it doesn't have Angular decorators, // thus no further processing is required. - if (!this.classCompilationMap.has(node)) return {node}; + if (!this.classCompilationMap.has(node)) { + return {node}; + } // There is at least one field to add. const statements: ts.Statement[] = []; const members = [...node.members]; - this.classCompilationMap.get(node)!.forEach(field => { + for (const field of this.classCompilationMap.get(node)!) { // Translate the initializer for the field into TS nodes. const exprNode = translateExpression( field.initializer, this.importManager, this.defaultImportRecorder, @@ -120,7 +122,7 @@ class IvyTransformationVisitor extends Visitor { .forEach(stmt => statements.push(stmt)); members.push(property); - }); + } // Replace the class declaration with an updated version. node = ts.updateClassDeclaration( diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 86a7696e141..f7cd3512102 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -6613,6 +6613,68 @@ export const Foo = Foo__PRE_R3__; const jsContents = env.getContents('test.js'); expect(jsContents).toContain('styles: ["h1[_ngcontent-%COMP%] {font-size: larger}"]'); }); + + it('should share same styles declared in different components in the same file', () => { + env.write('test.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'comp-a', + template: 'Comp A', + styles: [ + 'span { font-size: larger; }', + 'div { background: url(/some-very-very-long-path.png); }', + 'img { background: url(/a/some-very-very-long-path.png); }' + ] + }) + export class CompA {} + + @Component({ + selector: 'comp-b', + template: 'Comp B', + styles: [ + 'span { font-size: larger; }', + 'div { background: url(/some-very-very-long-path.png); }', + 'img { background: url(/b/some-very-very-long-path.png); }' + ] + }) + export class CompB {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + + // Verify that long styles present in both components are extracted to a separate var. + expect(jsContents) + .toContain( + '_c0 = "div[_ngcontent-%COMP%] { background: url(/some-very-very-long-path.png); }";'); + + expect(jsContents) + .toContain( + 'styles: [' + + // This style is present in both components, but was not extracted into a separate + // var since it doesn't reach length threshold (50 chars) in `ConstantPool`. + '"span[_ngcontent-%COMP%] { font-size: larger; }", ' + + // Style that is present in both components, but reaches length threshold - + // extracted to a separate var. + '_c0, ' + + // Style that is unique to this component, but that reaches length threshold - + // remains a string in the `styles` array. + '"img[_ngcontent-%COMP%] { background: url(/a/some-very-very-long-path.png); }"]'); + + expect(jsContents) + .toContain( + 'styles: [' + + // This style is present in both components, but was not extracted into a separate + // var since it doesn't reach length threshold (50 chars) in `ConstantPool`. + '"span[_ngcontent-%COMP%] { font-size: larger; }", ' + + // Style that is present in both components, but reaches length threshold - + // extracted to a separate var. + '_c0, ' + + // Style that is unique to this component, but that reaches length threshold - + // remains a string in the `styles` array. + '"img[_ngcontent-%COMP%] { background: url(/b/some-very-very-long-path.png); }"]'); + }); }); describe('non-exported classes', () => { diff --git a/packages/compiler/src/constant_pool.ts b/packages/compiler/src/constant_pool.ts index 24e800c95d1..b083a2c1ede 100644 --- a/packages/compiler/src/constant_pool.ts +++ b/packages/compiler/src/constant_pool.ts @@ -316,5 +316,5 @@ function isVariable(e: o.Expression): e is o.ReadVarExpr { function isLongStringExpr(expr: o.LiteralExpr): boolean { return typeof expr.value === 'string' && - expr.value.length > POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS; + expr.value.length >= POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS; } \ No newline at end of file diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index f0c305fe190..7adf55fb438 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -231,7 +231,7 @@ export function compileComponentFromMetadata( const styleValues = meta.encapsulation == core.ViewEncapsulation.Emulated ? compileStyles(meta.styles, CONTENT_ATTR, HOST_ATTR) : meta.styles; - const strings = styleValues.map(str => o.literal(str)); + const strings = styleValues.map(str => constantPool.getConstLiteral(o.literal(str))); definitionMap.set('styles', o.literalArr(strings)); } else if (meta.encapsulation === core.ViewEncapsulation.Emulated) { // If there is no style, don't generate css selectors on elements