From 8e5969bb52c791599c437e90248f7314785c5869 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 9d240aea2f..b91c2891d6 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": 450596, + "main-es2015": 448419, "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 c587ebcc15..99933680d0 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 9ad1a3a678..e5fac6559c 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -6623,6 +6623,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 24e800c95d..b083a2c1ed 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 f0c305fe19..7adf55fb43 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