diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index ccf6bb2eef..cae168aedb 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -1270,12 +1270,10 @@ describe('i18n support in the view compiler', () => { template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelementStart(0, "span", $_c0$); - $r3$.ɵi18nStart(1, $MSG_EXTERNAL_5295701706185791735$$APP_SPEC_TS_0$); - $r3$.ɵi18nEnd(); + $r3$.ɵi18n(1, $MSG_EXTERNAL_5295701706185791735$$APP_SPEC_TS_0$); $r3$.ɵelementEnd(); $r3$.ɵelementStart(2, "span", $_c1$); - $r3$.ɵi18nStart(3, $MSG_EXTERNAL_4722270221386399294$$APP_SPEC_TS_2$); - $r3$.ɵi18nEnd(); + $r3$.ɵi18n(3, $MSG_EXTERNAL_4722270221386399294$$APP_SPEC_TS_2$); $r3$.ɵelementEnd(); } } 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 f8d06cf28f..e93c462696 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 @@ -395,6 +395,99 @@ describe('compiler compliance: styling', () => { expectEmit(result.source, template, 'Incorrect template'); }); + it('should correctly count the total slots required when style/class bindings include interpolation', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-component-with-interpolation', + template: \` +
+ \` + }) + export class MyComponentWithInterpolation { + fooId = '123'; + } + + @Component({ + selector: 'my-component-with-muchos-interpolation', + template: \` +
+ \` + }) + export class MyComponentWithMuchosInterpolation { + fooId = '123'; + fooUsername = 'superfoo'; + } + + @Component({ + selector: 'my-component-without-interpolation', + template: \` +
+ \` + }) + export class MyComponentWithoutInterpolation { + exp = 'bar'; + } + + @NgModule({declarations: [MyComponentWithInterpolation, MyComponentWithMuchosInterpolation, MyComponentWithoutInterpolation]}) + export class MyModule {} + ` + } + }; + + const template = ` + … + consts: 1, + vars: 1, + template: function MyComponentWithInterpolation_Template(rf, $ctx$) { + if (rf & 1) { + $r3$.ɵelementStart(0, "div"); + $r3$.ɵelementStyling(); + $r3$.ɵelementEnd(); + } + if (rf & 2) { + $r3$.ɵelementStylingMap(0, $r3$.ɵinterpolation1("foo foo-", $ctx$.fooId, "")); + $r3$.ɵelementStylingApply(0); + } + } + … + consts: 1, + vars: 2, + template: function MyComponentWithMuchosInterpolation_Template(rf, $ctx$) { + if (rf & 1) { + $r3$.ɵelementStart(0, "div"); + $r3$.ɵelementStyling(); + $r3$.ɵelementEnd(); + } + if (rf & 2) { + $r3$.ɵelementStylingMap(0, $r3$.ɵinterpolation2("foo foo-", $ctx$.fooId, "-", $ctx$.fooUsername, "")); + $r3$.ɵelementStylingApply(0); + } + } + … + consts: 1, + vars: 0, + template: function MyComponentWithoutInterpolation_Template(rf, $ctx$) { + if (rf & 1) { + $r3$.ɵelementStart(0, "div"); + $r3$.ɵelementStyling(); + $r3$.ɵelementEnd(); + } + if (rf & 2) { + $r3$.ɵelementStylingMap(0, $ctx$.exp); + $r3$.ɵelementStylingApply(0); + } + } + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + it('should place initial, multi, singular and application followed by attribute style instructions in the template code in that order', () => { const files = { @@ -688,8 +781,7 @@ describe('compiler compliance: styling', () => { vars: 2, template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { - $r3$.ɵelementStart(0, "div", $e0_attrs$); - $r3$.ɵelementEnd(); + $r3$.ɵelement(0, "div", $e0_attrs$); } if (rf & 2) { $r3$.ɵelementAttribute(0, "class", $r3$.ɵbind("round")); diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index f8045eaaef..ceec9614be 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -747,7 +747,7 @@ function createHostBindingsFunction( createStatements.push(createStylingStmt(hostInstruction, bindingContext, bindingFn)); } - if (styleBuilder.hasBindingsOrInitialValues()) { + if (styleBuilder.hasBindings) { // singular style/class bindings (things like `[style.prop]` and `[class.name]`) // MUST be registered on a given element within the component/directive // templateFn/hostBindingsFn functions. The instruction below will figure out diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index 9eab564f8e..0b2c322f91 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -7,7 +7,7 @@ */ import {ConstantPool} from '../../constant_pool'; import {AttributeMarker} from '../../core'; -import {AST, BindingType} from '../../expression_parser/ast'; +import {AST, BindingType, Interpolation} from '../../expression_parser/ast'; import * as o from '../../output/output_ast'; import {ParseSourceSpan} from '../../parse_util'; import * as t from '../r3_ast'; @@ -23,6 +23,7 @@ import {ValueConverter} from './template'; export interface Instruction { sourceSpan: ParseSourceSpan|null; reference: o.ExternalReference; + allocateBindingSlots: number; buildParams(convertFn: (value: any) => o.Expression): o.Expression[]; } @@ -36,7 +37,6 @@ interface BoundStylingEntry { value: AST; } - /** * Produces creation/update instructions for all styling bindings (class and style) * @@ -73,7 +73,7 @@ export class StylingBuilder { * Whether or not there are any styling bindings present * (i.e. `[style]`, `[class]`, `[style.prop]` or `[class.name]`) */ - private _hasBindings = false; + public hasBindings = false; /** the input for [class] (if it exists) */ private _classMapInput: BoundStylingEntry|null = null; @@ -110,8 +110,6 @@ export class StylingBuilder { constructor(private _elementIndexExpr: o.Expression, private _directiveExpr: o.Expression|null) {} - hasBindingsOrInitialValues() { return this._hasBindings || this._hasInitialValues; } - /** * Registers a given input to the styling builder to be later used when producing AOT code. * @@ -158,7 +156,7 @@ export class StylingBuilder { this._styleMapInput = entry; } this._lastStylingInput = entry; - this._hasBindings = true; + this.hasBindings = true; return entry; } @@ -172,7 +170,7 @@ export class StylingBuilder { this._classMapInput = entry; } this._lastStylingInput = entry; - this._hasBindings = true; + this.hasBindings = true; return entry; } @@ -235,6 +233,7 @@ export class StylingBuilder { return { sourceSpan, reference: R3.elementHostAttrs, + allocateBindingSlots: 0, buildParams: () => { this.populateInitialStylingAttrs(attrs); return [this._directiveExpr !, getConstantLiteralFromArray(constantPool, attrs)]; @@ -252,9 +251,10 @@ export class StylingBuilder { */ buildElementStylingInstruction(sourceSpan: ParseSourceSpan|null, constantPool: ConstantPool): Instruction|null { - if (this._hasBindings) { + if (this.hasBindings) { return { sourceSpan, + allocateBindingSlots: 0, reference: R3.elementStyling, buildParams: () => { // a string array of every style-based binding @@ -315,18 +315,27 @@ export class StylingBuilder { buildElementStylingMapInstruction(valueConverter: ValueConverter): Instruction|null { if (this._classMapInput || this._styleMapInput) { const stylingInput = this._classMapInput ! || this._styleMapInput !; + let totalBindingSlotsRequired = 0; // these values must be outside of the update block so that they can // be evaluted (the AST visit call) during creation time so that any // pipes can be picked up in time before the template is built const mapBasedClassValue = this._classMapInput ? this._classMapInput.value.visit(valueConverter) : null; + if (mapBasedClassValue instanceof Interpolation) { + totalBindingSlotsRequired += mapBasedClassValue.expressions.length; + } + const mapBasedStyleValue = this._styleMapInput ? this._styleMapInput.value.visit(valueConverter) : null; + if (mapBasedStyleValue instanceof Interpolation) { + totalBindingSlotsRequired += mapBasedStyleValue.expressions.length; + } return { sourceSpan: stylingInput.sourceSpan, reference: R3.elementStylingMap, + allocateBindingSlots: totalBindingSlotsRequired, buildParams: (convertFn: (value: any) => o.Expression) => { const params: o.Expression[] = [this._elementIndexExpr]; @@ -356,12 +365,14 @@ export class StylingBuilder { private _buildSingleInputs( reference: o.ExternalReference, inputs: BoundStylingEntry[], mapIndex: Map, allowUnits: boolean, valueConverter: ValueConverter): Instruction[] { + let totalBindingSlotsRequired = 0; return inputs.map(input => { const bindingIndex: number = mapIndex.get(input.name) !; const value = input.value.visit(valueConverter); + totalBindingSlotsRequired += (value instanceof Interpolation) ? value.expressions.length : 0; return { sourceSpan: input.sourceSpan, - reference, + allocateBindingSlots: totalBindingSlotsRequired, reference, buildParams: (convertFn: (value: any) => o.Expression) => { const params = [this._elementIndexExpr, o.literal(bindingIndex), convertFn(value)]; if (allowUnits) { @@ -401,6 +412,7 @@ export class StylingBuilder { return { sourceSpan: this._lastStylingInput ? this._lastStylingInput.sourceSpan : null, reference: R3.elementStylingApply, + allocateBindingSlots: 0, buildParams: () => { const params: o.Expression[] = [this._elementIndexExpr]; if (this._directiveExpr) { @@ -417,7 +429,7 @@ export class StylingBuilder { */ buildUpdateLevelInstructions(valueConverter: ValueConverter) { const instructions: Instruction[] = []; - if (this._hasBindings) { + if (this.hasBindings) { const mapInstruction = this.buildElementStylingMapInstruction(valueConverter); if (mapInstruction) { instructions.push(mapInstruction); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 4f1f1762bf..7b31af58b5 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -602,11 +602,11 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver return element.children.length > 0; }; - const createSelfClosingInstruction = !stylingBuilder.hasBindingsOrInitialValues() && - !isNgContainer && element.outputs.length === 0 && i18nAttrs.length === 0 && !hasChildren(); + const createSelfClosingInstruction = !stylingBuilder.hasBindings && !isNgContainer && + element.outputs.length === 0 && i18nAttrs.length === 0 && !hasChildren(); const createSelfClosingI18nInstruction = !createSelfClosingInstruction && - !stylingBuilder.hasBindingsOrInitialValues() && hasTextChildrenOnly(element.children); + !stylingBuilder.hasBindings && hasTextChildrenOnly(element.children); if (createSelfClosingInstruction) { this.creationInstruction(element.sourceSpan, R3.element, trimTrailingNulls(parameters)); @@ -682,6 +682,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // `elementStylingMap`, `elementClassProp` and `elementStylingApply` are all generated // and assign in the code below. stylingBuilder.buildUpdateLevelInstructions(this._valueConverter).forEach(instruction => { + this._bindingSlots += instruction.allocateBindingSlots; this.processStylingInstruction(implicit, instruction, false); }); diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index 79deb6699a..fc1e046894 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -2915,11 +2915,10 @@ const DEFAULT_COMPONENT_ID = '1'; expect(cmp.log).toEqual(['parent-done', 'child-done']); })); - fixmeIvy( - 'FW-944 - style/class bindings lose track of consts/vars when interpolation is present') - .it('should fire callbacks and collect the correct the totalTime and element details for any queried sub animations', - fakeAsync(() => { - @Component({ + it('should fire callbacks and collect the correct the totalTime and element details for any queried sub animations', + fakeAsync( + () => { + @Component({ selector: 'my-cmp', template: `
@@ -2955,62 +2954,62 @@ const DEFAULT_COMPONENT_ID = '1'; ] }) class Cmp { - log: string[] = []; - events: {[name: string]: any} = {}; - // TODO(issue/24571): remove '!'. - exp !: string; - items: any = [0, 1, 2, 3]; + log: string[] = []; + events: {[name: string]: any} = {}; + // TODO(issue/24571): remove '!'. + exp !: string; + items: any = [0, 1, 2, 3]; - cb(name: string, phase: string, event: AnimationEvent) { - this.log.push(name + '-' + phase); - this.events[name] = event; - } - } + cb(name: string, phase: string, event: AnimationEvent) { + this.log.push(name + '-' + phase); + this.events[name] = event; + } + } - TestBed.configureTestingModule({declarations: [Cmp]}); + TestBed.configureTestingModule({declarations: [Cmp]}); - const engine = TestBed.get(ɵAnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; - cmp.exp = 'go'; - fixture.detectChanges(); - engine.flush(); - flushMicrotasks(); + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = 'go'; + fixture.detectChanges(); + engine.flush(); + flushMicrotasks(); - expect(cmp.log).toEqual(['c-0-start', 'c-1-start', 'c-2-start', 'c-3-start']); - cmp.log = []; + expect(cmp.log).toEqual(['c-0-start', 'c-1-start', 'c-2-start', 'c-3-start']); + cmp.log = []; - const players = getLog(); - // 1 + 4 + 4 = 9 players - expect(players.length).toEqual(9); + const players = getLog(); + // 1 + 4 + 4 = 9 players + expect(players.length).toEqual(9); - const [pA, pq1a, pq1b, pq1c, pq1d, pq2a, pq2b, pq2c, pq2d] = getLog(); - pA.finish(); - pq1a.finish(); - pq1b.finish(); - pq1c.finish(); - pq1d.finish(); - flushMicrotasks(); + const [pA, pq1a, pq1b, pq1c, pq1d, pq2a, pq2b, pq2c, pq2d] = getLog(); + pA.finish(); + pq1a.finish(); + pq1b.finish(); + pq1c.finish(); + pq1d.finish(); + flushMicrotasks(); - expect(cmp.log).toEqual([]); - pq2a.finish(); - pq2b.finish(); - pq2c.finish(); - pq2d.finish(); - flushMicrotasks(); + expect(cmp.log).toEqual([]); + pq2a.finish(); + pq2b.finish(); + pq2c.finish(); + pq2d.finish(); + flushMicrotasks(); - expect(cmp.log).toEqual( - ['all-done', 'c-0-done', 'c-1-done', 'c-2-done', 'c-3-done']); + expect(cmp.log).toEqual( + ['all-done', 'c-0-done', 'c-1-done', 'c-2-done', 'c-3-done']); - expect(cmp.events['c-0'].totalTime).toEqual(4100); // 1000 + 1000 + 1800 + 300 - expect(cmp.events['c-0'].element.innerText.trim()).toEqual('0'); - expect(cmp.events['c-1'].totalTime).toEqual(4100); - expect(cmp.events['c-1'].element.innerText.trim()).toEqual('1'); - expect(cmp.events['c-2'].totalTime).toEqual(4100); - expect(cmp.events['c-2'].element.innerText.trim()).toEqual('2'); - expect(cmp.events['c-3'].totalTime).toEqual(4100); - expect(cmp.events['c-3'].element.innerText.trim()).toEqual('3'); - })); + expect(cmp.events['c-0'].totalTime).toEqual(4100); // 1000 + 1000 + 1800 + 300 + expect(cmp.events['c-0'].element.innerText.trim()).toEqual('0'); + expect(cmp.events['c-1'].totalTime).toEqual(4100); + expect(cmp.events['c-1'].element.innerText.trim()).toEqual('1'); + expect(cmp.events['c-2'].totalTime).toEqual(4100); + expect(cmp.events['c-2'].element.innerText.trim()).toEqual('2'); + expect(cmp.events['c-3'].totalTime).toEqual(4100); + expect(cmp.events['c-3'].element.innerText.trim()).toEqual('3'); + })); }); describe('animation control flags', () => { diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index b144c2893d..4969036d82 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -295,73 +295,71 @@ import {HostListener} from '../../src/metadata/directives'; expect(p6.element.classList.contains('b3')).toBeTruthy(); }); - fixmeIvy( - 'FW-944 - style/class bindings lose track of consts/vars when interpolation is present') - .it('should be able to query all active animations using :animating in a query', () => { - @Component({ - selector: 'ani-cmp', - template: ` + it('should be able to query all active animations using :animating in a query', () => { + @Component({ + selector: 'ani-cmp', + template: `
`, - animations: [ - trigger( - 'myAnimation', - [ - transition( - '* => a', - [ - query( - '.item:nth-child(odd)', - [ - style({opacity: 0}), - animate(1000, style({opacity: 1})), - ]), - ]), - transition( - '* => b', - [ - query( - '.item:animating', - [ - style({opacity: 1}), - animate(1000, style({opacity: 0})), - ]), - ]), - ]), - ] - }) - class Cmp { - public exp: any; - public items: number[] = [0, 1, 2, 3, 4]; - } + animations: [ + trigger( + 'myAnimation', + [ + transition( + '* => a', + [ + query( + '.item:nth-child(odd)', + [ + style({opacity: 0}), + animate(1000, style({opacity: 1})), + ]), + ]), + transition( + '* => b', + [ + query( + '.item:animating', + [ + style({opacity: 1}), + animate(1000, style({opacity: 0})), + ]), + ]), + ]), + ] + }) + class Cmp { + public exp: any; + public items: number[] = [0, 1, 2, 3, 4]; + } - TestBed.configureTestingModule({declarations: [Cmp]}); + TestBed.configureTestingModule({declarations: [Cmp]}); - const engine = TestBed.get(ɵAnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; - cmp.exp = 'a'; - fixture.detectChanges(); - engine.flush(); + cmp.exp = 'a'; + fixture.detectChanges(); + engine.flush(); - let players = getLog(); - expect(players.length).toEqual(3); - resetLog(); + let players = getLog(); + expect(players.length).toEqual(3); + resetLog(); - cmp.exp = 'b'; - fixture.detectChanges(); - engine.flush(); + cmp.exp = 'b'; + fixture.detectChanges(); + engine.flush(); - players = getLog(); - expect(players.length).toEqual(3); - expect(players[0].element.classList.contains('e-0')).toBeTruthy(); - expect(players[1].element.classList.contains('e-2')).toBeTruthy(); - expect(players[2].element.classList.contains('e-4')).toBeTruthy(); - }); + players = getLog(); + expect(players.length).toEqual(3); + expect(players[0].element.classList.contains('e-0')).toBeTruthy(); + expect(players[1].element.classList.contains('e-2')).toBeTruthy(); + expect(players[2].element.classList.contains('e-4')).toBeTruthy(); + }); it('should be able to query all actively queued animation triggers via `@*:animating`', () => { diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index df30a7b3ee..4550a91a47 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -1916,6 +1916,30 @@ describe('render3 integration test', () => { fixture.update(); expect(target.style.getPropertyValue('width')).toEqual('777px'); }); + + it('should properly handle and render interpolation for class attribute bindings', () => { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div'); + elementStyling(); + elementEnd(); + } + if (rf & RenderFlags.Update) { + elementStylingMap(0, interpolation2('-', ctx.name, '-', ctx.age, '-')); + elementStylingApply(0); + } + }, 1, 2); + + const fixture = new ComponentFixture(App); + const target = fixture.hostElement.querySelector('div') !; + expect(target.classList.contains('-fred-36-')).toBeFalsy(); + + fixture.component.name = 'fred'; + fixture.component.age = '36'; + fixture.update(); + + expect(target.classList.contains('-fred-36-')).toBeTruthy(); + }); }); });