diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index 2935274601..d915929236 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -477,7 +477,7 @@ describe('compiler compliance', () => { const template = ` MyComponent.ɵcmp = i0.ɵɵdefineComponent({type:MyComponent,selectors:[["my-component"]], decls: 1, - vars: 2, + vars: 4, template: function MyComponent_Template(rf,ctx){ if (rf & 1) { $r3$.ɵɵelement(0, "div"); 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 ecc21689f3..ee8b92c419 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 @@ -377,8 +377,7 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵelement(0, "div"); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($ctx$.myStyleExp); + $r3$.ɵɵstyleMap($ctx$.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer); } } `; @@ -504,15 +503,14 @@ describe('compiler compliance: styling', () => { type: MyComponent, selectors:[["my-component"]], decls: 1, - vars: 5, + vars: 7, consts: [[${AttributeMarker.Styles}, "opacity", "1"]], template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { $r3$.ɵɵelement(0, "div", 0); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($ctx$.myStyleExp); + $r3$.ɵɵstyleMap($ctx$.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵstyleProp("width", $ctx$.myWidth)("height", $ctx$.myHeight); $r3$.ɵɵattribute("style", "border-width: 10px", $r3$.ɵɵsanitizeStyle); } @@ -551,14 +549,13 @@ describe('compiler compliance: styling', () => { type: MyComponent, selectors: [["my-component"]], decls: 1, - vars: 1, + vars: 2, template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelement(0, "div"); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleProp("background-image", ctx.myImage); + $r3$.ɵɵstyleProp("background-image", ctx.myImage, $r3$.ɵɵdefaultStyleSanitizer); } }, encapsulation: 2 @@ -697,7 +694,7 @@ describe('compiler compliance: styling', () => { type: MyComponent, selectors:[["my-component"]], decls: 1, - vars: 5, + vars: 7, consts: [[${AttributeMarker.Classes}, "grape"]], template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { @@ -816,8 +813,7 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵelement(0, "div"); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($ctx$.myStyleExp); + $r3$.ɵɵstyleMap($ctx$.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap($ctx$.myClassExp); } } @@ -858,8 +854,7 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 4, $ctx$.myStyleExp)); + $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 4, $ctx$.myStyleExp), $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 6, $ctx$.myClassExp)); } } @@ -911,11 +906,10 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { - $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))("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000)); - $r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 17, $ctx$.fooExp, 2000)); + $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 11, $ctx$.myStyleExp, 1000), $r3$.ɵɵdefaultStyleSanitizer); + $r3$.ɵɵclassMap($r3$.ɵɵpureFunction0(23, _c0)); + $r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 14, $ctx$.barExp, 3000))("baz", $r3$.ɵɵpipeBind2(3, 17, $ctx$.bazExp, 4000)); + $r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 20, $ctx$.fooExp, 2000)); $r3$.ɵɵadvance(5); $r3$.ɵɵtextInterpolate1(" ", $ctx$.item, ""); } @@ -1014,9 +1008,15 @@ describe('compiler compliance: styling', () => { hostAttrs: [${AttributeMarker.Classes}, "foo", "baz", ${AttributeMarker.Styles}, "width", "200px", "height", "500px"], hostVars: 6, hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { +<<<<<<< HEAD +======= + if (rf & 1) { + $r3$.ɵɵallocHostVars(8); + $r3$.ɵɵelementHostAttrs($e0_attrs$); + } +>>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.myStyle); + $r3$.ɵɵstyleMap(ctx.myStyle, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap(ctx.myClass); $r3$.ɵɵstyleProp("color", ctx.myColorProp); $r3$.ɵɵclassProp("foo", ctx.myFooClass); @@ -1070,9 +1070,14 @@ describe('compiler compliance: styling', () => { const template = ` hostVars: 8, hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { +<<<<<<< HEAD +======= + if (rf & 1) { + $r3$.ɵɵallocHostVars(12); + } +>>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.myStyle); + $r3$.ɵɵstyleMap(ctx.myStyle, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap(ctx.myClasses); $r3$.ɵɵstyleProp("height", ctx.myHeightProp, "pt")("width", ctx.myWidthProp); $r3$.ɵɵclassProp("bar", ctx.myBarClass)("foo", ctx.myFooClass); @@ -1129,8 +1134,7 @@ describe('compiler compliance: styling', () => { $r3$.ɵɵelement(0, "div"); } if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.myStyleExp); + $r3$.ɵɵstyleMap(ctx.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap(ctx.myClassExp); $r3$.ɵɵstyleProp("height", ctx.myHeightExp); $r3$.ɵɵclassProp("bar", ctx.myBarClassExp); @@ -1141,9 +1145,14 @@ describe('compiler compliance: styling', () => { const hostBindings = ` hostVars: 6, hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { +<<<<<<< HEAD +======= + if (rf & 1) { + $r3$.ɵɵallocHostVars(8); + } +>>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.myStyleExp); + $r3$.ɵɵstyleMap(ctx.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap(ctx.myClassExp); $r3$.ɵɵstyleProp("width", ctx.myWidthExp); $r3$.ɵɵclassProp("foo", ctx.myFooClassExp); @@ -1412,6 +1421,46 @@ describe('compiler compliance: styling', () => { expectEmit(result.source, template, 'Incorrect handling of interpolated style properties'); }); + it('should generate update instructions for interpolated style properties with a sanitizer', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: \` +
+ \` + }) + export class MyComponent { + myUrl1 = '...'; + myUrl2 = '...'; + myBoxX = '0px'; + myBoxY = '0px'; + myBoxWidth = '100px'; + myRepeat = 'no-repeat'; + } + ` + } + }; + + const template = ` + … + if (rf & 2) { + $r3$.ɵɵstylePropInterpolate1("background", "url(", ctx.myUrl1, ")", $r3$.ɵɵdefaultStyleSanitizer); + $r3$.ɵɵstylePropInterpolate2("border-image", "url(", ctx.myUrl2, ") ", ctx.myRepeat, " auto", $r3$.ɵɵdefaultStyleSanitizer); + $r3$.ɵɵstylePropInterpolate3("box-shadow", "", ctx.myBoxX, " ", ctx.myBoxY, " ", ctx.myBoxWidth, " black"); + } + … + `; + const result = compile(files, angularFiles); + + expectEmit(result.source, template, 'Incorrect handling of interpolated style properties'); + }); + it('should generate update instructions for interpolated style properties with !important', () => { const files = { @@ -1833,8 +1882,7 @@ describe('compiler compliance: styling', () => { hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 2) { $r3$.ɵɵhostProperty("id", ctx.id)("title", ctx.title); - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.myStyle); + $r3$.ɵɵstyleMap(ctx.myStyle, $r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵclassMap(ctx.myClass); } } @@ -1908,8 +1956,7 @@ describe('compiler compliance: styling', () => { template: function MyAppComp_Template(rf, ctx) { … if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleProp("background-image", ctx.bgExp); + $r3$.ɵɵstyleProp("background-image", ctx.bgExp, $r3$.ɵɵdefaultStyleSanitizer); } … } @@ -1943,8 +1990,7 @@ describe('compiler compliance: styling', () => { template: function MyAppComp_Template(rf, ctx) { … if (rf & 2) { - $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap(ctx.mapExp); + $r3$.ɵɵstyleMap(ctx.mapExp, $r3$.ɵɵdefaultStyleSanitizer); } … } @@ -2035,11 +2081,17 @@ describe('compiler compliance: styling', () => { const template = ` hostVars: 9, hostBindings: function MyDir_HostBindings(rf, ctx, elIndex) { +<<<<<<< HEAD +======= + … + $r3$.ɵɵallocHostVars(10); + … +>>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction if (rf & 2) { $r3$.ɵɵhostProperty("title", ctx.title); $r3$.ɵɵupdateSyntheticHostBinding("@anim", - $r3$.ɵɵpureFunction2(6, _c1, ctx._animValue, - $r3$.ɵɵpureFunction2(3, _c0, ctx._animParam1, ctx._animParam2))); + $r3$.ɵɵpureFunction2(7, _c1, ctx._animValue, + $r3$.ɵɵpureFunction2(4, _c0, ctx._animParam1, ctx._animParam2))); $r3$.ɵɵclassProp("foo", ctx.foo); } } diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 903986fd9f..c0d70a74cb 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -113,8 +113,6 @@ export class Identifiers { static stylePropInterpolateV: o.ExternalReference = {name: 'ɵɵstylePropInterpolateV', moduleName: CORE}; - static styleSanitizer: o.ExternalReference = {name: 'ɵɵstyleSanitizer', moduleName: CORE}; - static containerCreate: o.ExternalReference = {name: 'ɵɵcontainer', moduleName: CORE}; static nextContext: o.ExternalReference = {name: 'ɵɵnextContext', moduleName: CORE}; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index e7971b8cd3..2d0b49921b 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, StylingInstructionCall} from './styling_builder'; +import {MIN_STYLING_BINDING_SLOTS_REQUIRED, 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'; @@ -530,8 +530,6 @@ function createHostBindingsFunction( hostBindingsMetadata: R3HostMetadata, typeSourceSpan: ParseSourceSpan, bindingParser: BindingParser, constantPool: ConstantPool, selector: string, name: string, definitionMap: DefinitionMap): o.Expression|null { - // Initialize hostVarsCount to number of bound host properties (interpolations illegal) - const hostVarsCount = Object.keys(hostBindingsMetadata.properties).length; const elVarExp = o.variable('elIndex'); const bindingContext = o.variable(CONTEXT_NAME); const styleBuilder = new StylingBuilder(elVarExp, bindingContext); @@ -547,10 +545,38 @@ function createHostBindingsFunction( const createStatements: o.Statement[] = []; const updateStatements: o.Statement[] = []; - let totalHostVarsCount = hostVarsCount; const hostBindingSourceSpan = typeSourceSpan; const directiveSummary = metadataAsSummary(hostBindingsMetadata); + // Calculate host event bindings + const eventBindings = + bindingParser.createDirectiveHostEventAsts(directiveSummary, hostBindingSourceSpan); + if (eventBindings && eventBindings.length) { + const listeners = createHostListeners(eventBindings, name); + createStatements.push(...listeners); + } + + // Calculate the host property bindings + const bindings = bindingParser.createBoundHostProperties(directiveSummary, hostBindingSourceSpan); + const allOtherBindings: ParsedProperty[] = []; + + // We need to calculate the total amount of binding slots required by + // all the instructions together before any value conversions happen. + // Value conversions may require additional slots for interpolation and + // bindings with pipes. These calculates happen after this block. + let totalHostVarsCount = 0; + bindings && bindings.forEach((binding: ParsedProperty) => { + const name = binding.name; + const stylingInputWasSet = + styleBuilder.registerInputBasedOnName(name, binding.expression, binding.sourceSpan); + if (stylingInputWasSet) { + totalHostVarsCount += MIN_STYLING_BINDING_SLOTS_REQUIRED; + } else { + allOtherBindings.push(binding); + totalHostVarsCount++; + } + }); + let valueConverter: ValueConverter; const getValueConverter = () => { if (!valueConverter) { @@ -568,66 +594,50 @@ function createHostBindingsFunction( return valueConverter; }; - // Calculate host event bindings - const eventBindings = - bindingParser.createDirectiveHostEventAsts(directiveSummary, hostBindingSourceSpan); - if (eventBindings && eventBindings.length) { - const listeners = createHostListeners(eventBindings, name); - createStatements.push(...listeners); - } - - // Calculate the host property bindings - const bindings = bindingParser.createBoundHostProperties(directiveSummary, hostBindingSourceSpan); const propertyBindings: o.Expression[][] = []; const attributeBindings: o.Expression[][] = []; const syntheticHostBindings: o.Expression[][] = []; + allOtherBindings.forEach((binding: ParsedProperty) => { + // resolve literal arrays and literal objects + const value = binding.expression.visit(getValueConverter()); + const bindingExpr = bindingFn(bindingContext, value); - bindings && bindings.forEach((binding: ParsedProperty) => { - const name = binding.name; - const stylingInputWasSet = - styleBuilder.registerInputBasedOnName(name, binding.expression, binding.sourceSpan); - if (!stylingInputWasSet) { - // resolve literal arrays and literal objects - const value = binding.expression.visit(getValueConverter()); - const bindingExpr = bindingFn(bindingContext, value); + const {bindingName, instruction, isAttribute} = getBindingNameAndInstruction(binding); - const {bindingName, instruction, isAttribute} = getBindingNameAndInstruction(binding); + const securityContexts = + bindingParser.calcPossibleSecurityContexts(selector, bindingName, isAttribute) + .filter(context => context !== core.SecurityContext.NONE); - const securityContexts = - bindingParser.calcPossibleSecurityContexts(selector, bindingName, isAttribute) - .filter(context => context !== core.SecurityContext.NONE); - - let sanitizerFn: o.ExternalExpr|null = null; - if (securityContexts.length) { - if (securityContexts.length === 2 && - securityContexts.indexOf(core.SecurityContext.URL) > -1 && - securityContexts.indexOf(core.SecurityContext.RESOURCE_URL) > -1) { - // Special case for some URL attributes (such as "src" and "href") that may be a part - // of different security contexts. In this case we use special santitization function and - // select the actual sanitizer at runtime based on a tag name that is provided while - // invoking sanitization function. - sanitizerFn = o.importExpr(R3.sanitizeUrlOrResourceUrl); - } else { - sanitizerFn = resolveSanitizationFn(securityContexts[0], isAttribute); - } - } - const instructionParams = [o.literal(bindingName), bindingExpr.currValExpr]; - if (sanitizerFn) { - instructionParams.push(sanitizerFn); - } - - updateStatements.push(...bindingExpr.stmts); - - if (instruction === R3.hostProperty) { - propertyBindings.push(instructionParams); - } else if (instruction === R3.attribute) { - attributeBindings.push(instructionParams); - } else if (instruction === R3.updateSyntheticHostBinding) { - syntheticHostBindings.push(instructionParams); + let sanitizerFn: o.ExternalExpr|null = null; + if (securityContexts.length) { + if (securityContexts.length === 2 && + securityContexts.indexOf(core.SecurityContext.URL) > -1 && + securityContexts.indexOf(core.SecurityContext.RESOURCE_URL) > -1) { + // Special case for some URL attributes (such as "src" and "href") that may be a part + // of different security contexts. In this case we use special santitization function and + // select the actual sanitizer at runtime based on a tag name that is provided while + // invoking sanitization function. + sanitizerFn = o.importExpr(R3.sanitizeUrlOrResourceUrl); } else { - updateStatements.push(o.importExpr(instruction).callFn(instructionParams).toStmt()); + sanitizerFn = resolveSanitizationFn(securityContexts[0], isAttribute); } } + const instructionParams = [o.literal(bindingName), bindingExpr.currValExpr]; + if (sanitizerFn) { + instructionParams.push(sanitizerFn); + } + + updateStatements.push(...bindingExpr.stmts); + + if (instruction === R3.hostProperty) { + propertyBindings.push(instructionParams); + } else if (instruction === R3.attribute) { + attributeBindings.push(instructionParams); + } else if (instruction === R3.updateSyntheticHostBinding) { + syntheticHostBindings.push(instructionParams); + } else { + updateStatements.push(o.importExpr(instruction).callFn(instructionParams).toStmt()); + } }); if (propertyBindings.length > 0) { @@ -664,7 +674,8 @@ function createHostBindingsFunction( 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); + totalHostVarsCount += + Math.max(call.allocateBindingSlots - MIN_STYLING_BINDING_SLOTS_REQUIRED, 0); calls.push(convertStylingCall(call, bindingContext, bindingFn)); }); diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index e05df038ce..96a701eb3a 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -20,6 +20,56 @@ import {DefinitionMap, getInterpolationArgsLength} from './util'; const IMPORTANT_FLAG = '!important'; +/** + * Minimum amount of binding slots required in the runtime for style/class bindings. + * + * Styling in Angular uses up two slots in the runtime LView/TData data structures to + * record binding data, property information and metadata. + * + * When a binding is registered it will place the following information in the `LView`: + * + * slot 1) binding value + * slot 2) cached value (all other values collected before it in string form) + * + * When a binding is registered it will place the following information in the `TData`: + * + * slot 1) prop name + * slot 2) binding index that points to the previous style/class binding (and some extra config + * values) + * + * Let's imagine we have a binding that looks like so: + * + * ``` + *