diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index 23fff07a73..46083c897c 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -513,8 +513,7 @@ describe('compiler compliance: styling', () => { if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵstyleMap($ctx$.myStyleExp); - $r3$.ɵɵstyleProp("width", $ctx$.myWidth); - $r3$.ɵɵstyleProp("height", $ctx$.myHeight); + $r3$.ɵɵstyleProp("width", $ctx$.myWidth)("height", $ctx$.myHeight); $r3$.ɵɵattribute("style", "border-width: 10px", $r3$.ɵɵsanitizeStyle); } }, @@ -706,8 +705,7 @@ describe('compiler compliance: styling', () => { } if (rf & 2) { $r3$.ɵɵclassMap($ctx$.myClassExp); - $r3$.ɵɵclassProp("apple", $ctx$.yesToApple); - $r3$.ɵɵclassProp("orange", $ctx$.yesToOrange); + $r3$.ɵɵclassProp("apple", $ctx$.yesToApple)("orange", $ctx$.yesToOrange); $r3$.ɵɵattribute("class", "banana"); } }, @@ -916,8 +914,7 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 8, $ctx$.myStyleExp, 1000)); $r3$.ɵɵclassMap($r3$.ɵɵpureFunction0(20, _c0)); - $r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 11, $ctx$.barExp, 3000)); - $r3$.ɵɵstyleProp("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000)); + $r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 11, $ctx$.barExp, 3000))("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000)); $r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 17, $ctx$.fooExp, 2000)); $r3$.ɵɵadvance(5); $r3$.ɵɵtextInterpolate1(" ", $ctx$.item, ""); @@ -1081,10 +1078,8 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵstyleMap(ctx.myStyle); $r3$.ɵɵclassMap(ctx.myClasses); - $r3$.ɵɵstyleProp("height", ctx.myHeightProp, "pt"); - $r3$.ɵɵstyleProp("width", ctx.myWidthProp); - $r3$.ɵɵclassProp("bar", ctx.myBarClass); - $r3$.ɵɵclassProp("foo", ctx.myFooClass); + $r3$.ɵɵstyleProp("height", ctx.myHeightProp, "pt")("width", ctx.myWidthProp); + $r3$.ɵɵclassProp("bar", ctx.myBarClass)("foo", ctx.myFooClass); } }, decls: 0, @@ -1461,6 +1456,351 @@ describe('compiler compliance: styling', () => { }); + describe('instruction chaining', () => { + it('should chain classProp instruction calls', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`
\` + }) + export class MyComponent { + yesToApple = true; + yesToOrange = true; + tesToTomato = false; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵclassProp("apple", $ctx$.yesToApple)("orange", $ctx$.yesToOrange)("tomato", $ctx$.yesToTomato); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should chain styleProp instruction calls', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + color = 'red'; + border = '1px solid purple'; + transition = 'all 1337ms ease'; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstyleProp("color", $ctx$.color)("border", $ctx$.border)("transition", $ctx$.transition); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should chain mixed styleProp and classProp calls', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + color = 'red'; + border = '1px solid purple'; + transition = 'all 1337ms ease'; + yesToApple = true; + yesToOrange = true; + tesToTomato = false; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstyleProp("color", $ctx$.color)("border", $ctx$.border)("transition", $ctx$.transition); + $r3$.ɵɵclassProp("apple", $ctx$.yesToApple)("orange", $ctx$.yesToOrange)("tomato", $ctx$.yesToTomato); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should chain style interpolations of the same kind', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstylePropInterpolate1("color", "a", ctx.one, "b")("border", "a", ctx.one, "b")("transition", "a", ctx.one, "b"); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should chain style interpolations of multiple kinds', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstylePropInterpolate1("color", "a", ctx.one, "b")("border", "a", ctx.one, "b"); + $r3$.ɵɵstylePropInterpolate2("transition", "a", ctx.one, "b", ctx.two, "c")("width", "a", ctx.one, "b", ctx.two, "c"); + $r3$.ɵɵstylePropInterpolate3("height", "a", ctx.one, "b", ctx.two, "c", ctx.three, "d")("top", "a", ctx.one, "b", ctx.two, "c", ctx.three, "d"); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should break into multiple chains if there are other styling instructions in between', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + transition = 'all 1337ms ease'; + width = '42px'; + yesToApple = true; + yesToOrange = true; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstylePropInterpolate1("color", "a", ctx.one, "b")("border", "a", ctx.one, "b"); + $r3$.ɵɵstyleProp("transition", ctx.transition)("width", ctx.width); + $r3$.ɵɵstylePropInterpolate1("height", "a", ctx.one, "b")("top", "a", ctx.one, "b"); + $r3$.ɵɵclassProp("apple", ctx.yesToApple)("orange", ctx.yesToOrange); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should break into multiple chains if there are other styling interpolation instructions in between', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \`\` + }) + export class MyComponent { + transition = 'all 1337ms ease'; + width = '42px'; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + template: function MyComponent_Template(rf, $ctx$) { + … + if (rf & 2) { + $r3$.ɵɵstylePropInterpolate1("color", "a", ctx.one, "b")("border", "a", ctx.one, "b"); + $r3$.ɵɵstylePropInterpolate2("transition", "a", ctx.one, "b", ctx.two, "c"); + $r3$.ɵɵstylePropInterpolate3("width", "a", ctx.one, "b", ctx.two, "c", ctx.three, "d"); + $r3$.ɵɵstylePropInterpolate1("height", "a", ctx.one, "b")("top", "a", ctx.one, "b"); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should chain styling instructions inside host bindings', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, HostBinding} from '@angular/core'; + + @Component({ + template: '', + host: { + '[class.apple]': 'yesToApple', + '[style.color]': 'color', + '[class.tomato]': 'yesToTomato', + '[style.transition]': 'transition' + } + }) + export class MyComponent { + color = 'red'; + transition = 'all 1337ms ease'; + yesToApple = true; + tesToTomato = false; + + @HostBinding('style.border') + border = '1px solid purple'; + + @HostBinding('class.orange') + yesToOrange = true; + } + ` + } + }; + + const template = ` + … + MyComponent.ɵcmp = $r3$.ɵɵdefineComponent({ + … + hostBindings: function MyComponent_HostBindings(rf, $ctx$, elIndex) { + … + if (rf & 2) { + $r3$.ɵɵstyleProp("color", $ctx$.color)("transition", $ctx$.transition)("border", $ctx$.border); + $r3$.ɵɵclassProp("apple", $ctx$.yesToApple)("tomato", $ctx$.yesToTomato)("orange", $ctx$.yesToOrange); + } + }, + … + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + }); + it('should count only non-style and non-class host bindings on Components', () => { const files = { app: { diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index b589276cbc..c4b7ca3f2a 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -28,7 +28,7 @@ import {Render3ParseResult} from '../r3_template_transform'; import {prepareSyntheticListenerFunctionName, prepareSyntheticPropertyName, typeWithParameters} from '../util'; import {R3ComponentDef, R3ComponentMetadata, R3DirectiveDef, R3DirectiveMetadata, R3HostMetadata, R3QueryMetadata} from './api'; -import {StylingBuilder, StylingInstruction} from './styling_builder'; +import {StylingBuilder, StylingInstructionCall} from './styling_builder'; import {BindingScope, TemplateDefinitionBuilder, ValueConverter, makeBindingParser, prepareEventListenerParameters, renderFlagCheckIfStmt, resolveSanitizationFn} from './template'; import {CONTEXT_NAME, DefinitionMap, RENDER_FLAGS, TEMPORARY_NAME, asLiteral, chainedInstruction, conditionallyCreateMapObjectLiteral, getQueryPredicate, temporaryAllocator} from './util'; @@ -652,8 +652,12 @@ function createHostBindingsFunction( // collected earlier. const hostAttrs = convertAttributesToExpressions(hostBindingsMetadata.attributes); const hostInstruction = styleBuilder.buildHostAttrsInstruction(null, hostAttrs, constantPool); - if (hostInstruction) { - createStatements.push(createStylingStmt(hostInstruction, bindingContext, bindingFn)); + if (hostInstruction && hostInstruction.calls.length > 0) { + createStatements.push( + chainedInstruction( + hostInstruction.reference, + hostInstruction.calls.map(call => convertStylingCall(call, bindingContext, bindingFn))) + .toStmt()); } if (styleBuilder.hasBindings) { @@ -661,11 +665,18 @@ function createHostBindingsFunction( // the update block of a component/directive templateFn/hostBindingsFn so that the bindings // are evaluated and updated for the element. styleBuilder.buildUpdateLevelInstructions(getValueConverter()).forEach(instruction => { - // we subtract a value of `1` here because the binding slot was already - // allocated at the top of this method when all the input bindings were - // counted. - totalHostVarsCount += Math.max(instruction.allocateBindingSlots - 1, 0); - updateStatements.push(createStylingStmt(instruction, bindingContext, bindingFn)); + if (instruction.calls.length > 0) { + const calls: o.Expression[][] = []; + + instruction.calls.forEach(call => { + // we subtract a value of `1` here because the binding slot was already allocated + // at the top of this method when all the input bindings were counted. + totalHostVarsCount += Math.max(call.allocateBindingSlots - 1, 0); + calls.push(convertStylingCall(call, bindingContext, bindingFn)); + }); + + updateStatements.push(chainedInstruction(instruction.reference, calls).toStmt()); + } }); } @@ -699,12 +710,9 @@ function bindingFn(implicit: any, value: AST) { null, implicit, value, 'b', BindingForm.TrySimple, () => error('Unexpected interpolation')); } -function createStylingStmt( - instruction: StylingInstruction, bindingContext: any, bindingFn: Function): o.Statement { - const params = instruction.params(value => bindingFn(bindingContext, value).currValExpr); - return o.importExpr(instruction.reference, null, instruction.sourceSpan) - .callFn(params, instruction.sourceSpan) - .toStmt(); +function convertStylingCall( + call: StylingInstructionCall, bindingContext: any, bindingFn: Function) { + return call.params(value => bindingFn(bindingContext, value).currValExpr); } function getBindingNameAndInstruction(binding: ParsedProperty): diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index 65af1e9171..67d546ecab 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -11,7 +11,6 @@ import {AST, ASTWithSource, BindingPipe, BindingType, Interpolation} from '../.. import * as o from '../../output/output_ast'; import {ParseSourceSpan} from '../../parse_util'; import {isEmptyExpression} from '../../template_parser/template_parser'; -import {error} from '../../util'; import * as t from '../r3_ast'; import {Identifiers as R3} from '../r3_identifiers'; @@ -25,10 +24,15 @@ const IMPORTANT_FLAG = '!important'; * A styling expression summary that is to be processed by the compiler */ export interface StylingInstruction { - sourceSpan: ParseSourceSpan|null; reference: o.ExternalReference; - allocateBindingSlots: number; + /** Calls to individual styling instructions. Used when chaining calls to the same instruction. */ + calls: StylingInstructionCall[]; +} + +export interface StylingInstructionCall { + sourceSpan: ParseSourceSpan|null; supportsInterpolation?: boolean; + allocateBindingSlots: number; params: ((convertFn: (value: any) => o.Expression | o.Expression[]) => o.Expression[]); } @@ -280,17 +284,19 @@ export class StylingBuilder { constantPool: ConstantPool): StylingInstruction|null { if (this._directiveExpr && (attrs.length || this._hasInitialValues)) { return { - sourceSpan, reference: R3.elementHostAttrs, - allocateBindingSlots: 0, - params: () => { - // params => elementHostAttrs(attrs) - this.populateInitialStylingAttrs(attrs); - const attrArray = !attrs.some(attr => attr instanceof o.WrappedNodeExpr) ? - getConstantLiteralFromArray(constantPool, attrs) : - o.literalArr(attrs); - return [attrArray]; - } + calls: [{ + sourceSpan, + allocateBindingSlots: 0, + params: () => { + // params => elementHostAttrs(attrs) + this.populateInitialStylingAttrs(attrs); + const attrArray = !attrs.some(attr => attr instanceof o.WrappedNodeExpr) ? + getConstantLiteralFromArray(constantPool, attrs) : + o.literalArr(attrs); + return [attrArray]; + } + }] }; } return null; @@ -344,14 +350,16 @@ export class StylingBuilder { } return { - sourceSpan: stylingInput.sourceSpan, reference, - allocateBindingSlots: totalBindingSlotsRequired, - supportsInterpolation: isClassBased, - params: (convertFn: (value: any) => o.Expression | o.Expression[]) => { - const convertResult = convertFn(mapValue); - return Array.isArray(convertResult) ? convertResult : [convertResult]; - } + calls: [{ + supportsInterpolation: isClassBased, + sourceSpan: stylingInput.sourceSpan, + allocateBindingSlots: totalBindingSlotsRequired, + params: (convertFn: (value: any) => o.Expression | o.Expression[]) => { + const convertResult = convertFn(mapValue); + return Array.isArray(convertResult) ? convertResult : [convertResult]; + } + }] }; } @@ -360,25 +368,27 @@ export class StylingBuilder { allowUnits: boolean, valueConverter: ValueConverter, getInterpolationExpressionFn?: (value: Interpolation) => o.ExternalReference): StylingInstruction[] { - let totalBindingSlotsRequired = 0; - return inputs.map(input => { - const value = input.value.visit(valueConverter); + const instructions: StylingInstruction[] = []; - // each styling binding value is stored in the LView - let totalBindingSlotsRequired = 1; + inputs.forEach(input => { + const previousInstruction: StylingInstruction|undefined = + instructions[instructions.length - 1]; + const value = input.value.visit(valueConverter); + let referenceForCall = reference; + let totalBindingSlotsRequired = 1; // each styling binding value is stored in the LView if (value instanceof Interpolation) { totalBindingSlotsRequired += value.expressions.length; if (getInterpolationExpressionFn) { - reference = getInterpolationExpressionFn(value); + referenceForCall = getInterpolationExpressionFn(value); } } - return { + const call = { sourceSpan: input.sourceSpan, + allocateBindingSlots: totalBindingSlotsRequired, supportsInterpolation: !!getInterpolationExpressionFn, - allocateBindingSlots: totalBindingSlotsRequired, reference, params: (convertFn: (value: any) => o.Expression | o.Expression[]) => { // params => stylingProp(propName, value) const params: o.Expression[] = []; @@ -398,7 +408,20 @@ export class StylingBuilder { return params; } }; + + // If we ended up generating a call to the same instruction as the previous styling property + // we can chain the calls together safely to save some bytes, otherwise we have to generate + // a separate instruction call. This is primarily a concern with interpolation instructions + // where we may start off with one `reference`, but end up using another based on the + // number of interpolations. + if (previousInstruction && previousInstruction.reference === referenceForCall) { + previousInstruction.calls.push(call); + } else { + instructions.push({reference: referenceForCall, calls: [call]}); + } }); + + return instructions; } private _buildClassInputs(valueConverter: ValueConverter): StylingInstruction[] { @@ -420,10 +443,12 @@ export class StylingBuilder { private _buildSanitizerFn(): StylingInstruction { return { - sourceSpan: this._firstStylingInput ? this._firstStylingInput.sourceSpan : null, reference: R3.styleSanitizer, - allocateBindingSlots: 0, - params: () => [o.importExpr(R3.defaultStyleSanitizer)] + calls: [{ + sourceSpan: this._firstStylingInput ? this._firstStylingInput.sourceSpan : null, + allocateBindingSlots: 0, + params: () => [o.importExpr(R3.defaultStyleSanitizer)] + }] }; } @@ -476,20 +501,6 @@ function getConstantLiteralFromArray( return values.length ? constantPool.getConstLiteral(o.literalArr(values), true) : o.NULL_EXPR; } -/** - * Simple helper function that adds a parameter or does nothing at all depending on the provided - * predicate and totalExpectedArgs values - */ -function addParam( - params: o.Expression[], predicate: any, value: o.Expression | null, argNumber: number, - totalExpectedArgs: number) { - if (predicate && value) { - params.push(value); - } else if (argNumber < totalExpectedArgs) { - params.push(o.NULL_EXPR); - } -} - export function parseProperty(name: string): {property: string, unit: string, hasOverrideFlag: boolean} { let hasOverrideFlag = false; diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 9b0b5bd9ea..957935241d 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -708,8 +708,7 @@ export class TemplateDefinitionBuilder implements t.Visitor