diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts index 98dfb40fcd..8f484a3284 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts @@ -122,7 +122,7 @@ describe('r3_view_compiler', () => { }); describe('animations', () => { - it('should keep @attr but suppress [@attr]', () => { + it('should not register any @attr attributes as static attributes', () => { const files: MockDirectory = { app: { 'example.ts': ` @@ -130,7 +130,7 @@ describe('r3_view_compiler', () => { @Component({ selector: 'my-app', - template: '
' + template: '
' }) export class MyApp { } @@ -141,14 +141,14 @@ describe('r3_view_compiler', () => { }; const template = ` - const _c0 = ["@attrOnly", ""]; - // ... template: function MyApp_Template(rf, ctx) { if (rf & 1) { - $i0$.ɵelement(0, "div", _c0); - // ... + $i0$.ɵelement(0, "div"); + } + if (rf & 2) { + $i0$.ɵelementProperty(0, "@attr", …); + $i0$.ɵelementProperty(0, "@binding", …); } - // ... }`; const result = compile(files, angularFiles); expectEmit(result.source, template, 'Incorrect initialization attributes'); @@ -173,14 +173,12 @@ describe('r3_view_compiler', () => { }; const template = ` - const _c0 = [3, "@mySelector"]; - // ... template: function MyApp_Template(rf, ctx) { if (rf & 1) { - $i0$.ɵelementStart(0, "div", _c0); - // ... + $i0$.ɵelementStart(0, "div"); + … + $i0$.ɵelementProperty(0, "@mySelector", …); } - // ... }`; const result = compile(files, angularFiles); expectEmit(result.source, template, 'Incorrect initialization attributes'); 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 e93c462696..9f60dd4556 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 @@ -214,21 +214,21 @@ describe('compiler compliance: styling', () => { }; const template = ` - const $e1_attrs$ = ["@bar", ""]; - const $e2_attrs$ = ["@baz", ""]; … MyComponent.ngComponentDef = $r3$.ɵdefineComponent({ … consts: 3, - vars: 1, + vars: 3, template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { $r3$.ɵelement(0, "div"); - $r3$.ɵelement(1, "div", $e1_attrs$); - $r3$.ɵelement(2, "div", $e2_attrs$); + $r3$.ɵelement(1, "div"); + $r3$.ɵelement(2, "div"); } if (rf & 2) { $r3$.ɵelementProperty(0, "@foo", $r3$.ɵbind(ctx.exp)); + $r3$.ɵelementProperty(1, "@bar", $r3$.ɵbind(undefined)); + $r3$.ɵelementProperty(2, "@baz", $r3$.ɵbind(undefined)); } }, encapsulation: 2 @@ -280,7 +280,7 @@ describe('compiler compliance: styling', () => { vars: 1, template: function MyComponent_Template(rf, ctx) { if (rf & 1) { - $r3$.ɵelementStart(0, "div", _c0); + $r3$.ɵelementStart(0, "div"); $r3$.ɵlistener("@myAnimation.start", function MyComponent_Template_div_animation_myAnimation_start_0_listener($event) { return ctx.onStart($event); }); $r3$.ɵlistener("@myAnimation.done", function MyComponent_Template_div_animation_myAnimation_done_0_listener($event) { return ctx.onDone($event); }); $r3$.ɵelementEnd(); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 7b31af58b5..7d0fb1db38 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -29,7 +29,7 @@ import {error} from '../../util'; import * as t from '../r3_ast'; import {Identifiers as R3} from '../r3_identifiers'; import {htmlAstToRender3Ast} from '../r3_template_transform'; -import {getSyntheticPropertyName, prepareSyntheticListenerFunctionName, prepareSyntheticListenerName, prepareSyntheticPropertyName} from '../util'; +import {prepareSyntheticListenerFunctionName, prepareSyntheticListenerName, prepareSyntheticPropertyName} from '../util'; import {R3QueryMetadata} from './api'; import {I18nContext} from './i18n/context'; @@ -571,8 +571,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // this will build the instructions so that they fall into the following syntax // add attributes for directive matching purposes - attributes.push(...this.prepareSyntheticAndSelectOnlyAttrs( - allOtherInputs, element.outputs, stylingBuilder)); + attributes.push( + ...this.prepareSelectOnlyAttrs(allOtherInputs, element.outputs, stylingBuilder)); parameters.push(this.toAttrsParam(attributes)); // local refs (ex.:
) @@ -686,8 +686,14 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.processStylingInstruction(implicit, instruction, false); }); + // the reason why `undefined` is used is because the renderer understands this as a + // special value to symbolize that there is no RHS to this binding + // TODO (matsko): revisit this once FW-959 is approached + const emptyValueBindInstruction = o.importExpr(R3.bind).callFn([o.literal(undefined)]); + // Generate element input bindings allOtherInputs.forEach((input: t.BoundAttribute) => { + const instruction = mapBindingToInstruction(input.type); if (input.type === BindingType.Animation) { const value = input.value.visit(this._valueConverter); @@ -696,21 +702,19 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // 2. [@binding]="{value:fooExp, params:{...}}" // 3. [@binding] // 4. @binding - // only formats 1. and 2. include the actual binding of a value to - // an expression and therefore only those should be the only two that - // are allowed. The check below ensures that a binding with no expression - // does not get an empty `elementProperty` instruction created for it. - const hasValue = value && (value instanceof LiteralPrimitive) ? !!value.value : true; - if (hasValue) { - this.allocateBindingSlots(value); - const bindingName = prepareSyntheticPropertyName(input.name); - this.updateInstruction(input.sourceSpan, R3.elementProperty, () => { - return [ - o.literal(elementIndex), o.literal(bindingName), - this.convertPropertyBinding(implicit, value) - ]; - }); - } + // All formats will be valid for when a synthetic binding is created. + // The reasoning for this is because the renderer should get each + // synthetic binding value in the order of the array that they are + // defined in... + const hasValue = value instanceof LiteralPrimitive ? !!value.value : true; + this.allocateBindingSlots(value); + const bindingName = prepareSyntheticPropertyName(input.name); + this.updateInstruction(input.sourceSpan, R3.elementProperty, () => { + return [ + o.literal(elementIndex), o.literal(bindingName), + (hasValue ? this.convertPropertyBinding(implicit, value) : emptyValueBindInstruction) + ]; + }); } else if (instruction) { const value = input.value.visit(this._valueConverter); if (value !== undefined) { @@ -775,7 +779,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const attrsExprs: o.Expression[] = []; template.attributes.forEach( (a: t.TextAttribute) => { attrsExprs.push(asLiteral(a.name), asLiteral(a.value)); }); - attrsExprs.push(...this.prepareSyntheticAndSelectOnlyAttrs(template.inputs, template.outputs)); + attrsExprs.push(...this.prepareSelectOnlyAttrs(template.inputs, template.outputs)); parameters.push(this.toAttrsParam(attrsExprs)); // local refs (ex.: ) @@ -969,7 +973,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver return originalSlots; } - private allocateBindingSlots(value: AST) { + private allocateBindingSlots(value: AST|null) { this._bindingSlots += value instanceof Interpolation ? value.expressions.length : 1; } @@ -1018,21 +1022,15 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver * STYLES, style1, value1, style2, value2, * SELECT_ONLY, name1, name2, name2, ...] * ``` + * + * Note that this function will fully ignore all synthetic (@foo) attribute values + * because those values are intended to always be generated as property instructions. */ - private prepareSyntheticAndSelectOnlyAttrs( + private prepareSelectOnlyAttrs( inputs: t.BoundAttribute[], outputs: t.BoundEvent[], styles?: StylingBuilder): o.Expression[] { - const attrExprs: o.Expression[] = []; - const nonSyntheticInputs: t.BoundAttribute[] = []; const alreadySeen = new Set(); - - function isASTWithSource(ast: AST): ast is ASTWithSource { - return ast instanceof ASTWithSource; - } - - function isLiteralPrimitive(ast: AST): ast is LiteralPrimitive { - return ast instanceof LiteralPrimitive; - } + const attrExprs: o.Expression[] = []; function addAttrExpr(key: string | number, value?: o.Expression): void { if (typeof key === 'string') { @@ -1048,27 +1046,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } } - if (inputs.length) { - const EMPTY_STRING_EXPR = asLiteral(''); - inputs.forEach(input => { - if (input.type === BindingType.Animation) { - // @attributes are for Renderer2 animation @triggers, but this feature - // may be supported differently in future versions of angular. However, - // @triggers should always just be treated as regular attributes (it's up - // to the renderer to detect and use them in a special way). - const valueExp = input.value; - if (isASTWithSource(valueExp)) { - const literal = valueExp.ast; - if (isLiteralPrimitive(literal) && literal.value === undefined) { - addAttrExpr(prepareSyntheticPropertyName(input.name), EMPTY_STRING_EXPR); - } - } - } else { - nonSyntheticInputs.push(input); - } - }); - } - // it's important that this occurs before SelectOnly because once `elementStart` // comes across the SelectOnly marker then it will continue reading each value as // as single property value cell by cell. @@ -1076,14 +1053,30 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver styles.populateInitialStylingAttrs(attrExprs); } - if (nonSyntheticInputs.length || outputs.length) { - addAttrExpr(core.AttributeMarker.SelectOnly); - nonSyntheticInputs.forEach((i: t.BoundAttribute) => addAttrExpr(i.name)); - outputs.forEach((o: t.BoundEvent) => { - const name = - o.type === ParsedEventType.Animation ? getSyntheticPropertyName(o.name) : o.name; - addAttrExpr(name); - }); + if (inputs.length || outputs.length) { + const attrsStartIndex = attrExprs.length; + + for (let i = 0; i < inputs.length; i++) { + const input = inputs[i]; + if (input.type !== BindingType.Animation) { + addAttrExpr(input.name); + } + } + + for (let i = 0; i < outputs.length; i++) { + const output = outputs[i]; + if (output.type !== ParsedEventType.Animation) { + addAttrExpr(output.name); + } + } + + // this is a cheap way of adding the marker only after all the input/output + // values have been filtered (by not including the animation ones) and added + // to the expressions. The marker is important because it tells the runtime + // code that this is where attributes without values start... + if (attrExprs.length) { + attrExprs.splice(attrsStartIndex, 0, o.literal(core.AttributeMarker.SelectOnly)); + } } return attrExprs; diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index fc1e046894..e86b7be998 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -1569,64 +1569,61 @@ const DEFAULT_COMPONENT_ID = '1'; } }); - fixmeIvy( - 'FW-932: Animation @triggers are not reported to the renderer in Ivy as they are in VE') - .it('should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles', - () => { - @Component({ - selector: 'ani-cmp', - template: ` + it('should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles', + () => { + @Component({ + selector: 'ani-cmp', + template: `
`, - animations: [ - trigger('trig1', [transition( - 'state => void', [animate(1000, style({opacity: 0}))])]), - trigger( - 'trig2', [transition(':leave', [animate(1000, style({width: '0px'}))])]) - ] - }) - class Cmp { - public exp = true; - public exp2 = 'state'; - } + animations: [ + trigger( + 'trig1', [transition('state => void', [animate(1000, style({opacity: 0}))])]), + trigger('trig2', [transition(':leave', [animate(1000, style({width: '0px'}))])]) + ] + }) + class Cmp { + public exp = true; + public exp2 = 'state'; + } - TestBed.configureTestingModule({declarations: [Cmp]}); + TestBed.configureTestingModule({declarations: [Cmp]}); - const engine = TestBed.get(ɵAnimationEngine); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; - cmp.exp = true; - fixture.detectChanges(); - engine.flush(); - resetLog(); + const engine = TestBed.get(ɵAnimationEngine); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + cmp.exp = true; + fixture.detectChanges(); + engine.flush(); + resetLog(); - const element = getDOM().querySelector(fixture.nativeElement, '.ng-if'); - assertHasParent(element, true); + const element = getDOM().querySelector(fixture.nativeElement, '.ng-if'); + assertHasParent(element, true); - cmp.exp = false; - fixture.detectChanges(); - engine.flush(); + cmp.exp = false; + fixture.detectChanges(); + engine.flush(); - assertHasParent(element, true); + assertHasParent(element, true); - expect(getLog().length).toEqual(2); + expect(getLog().length).toEqual(2); - const player2 = getLog().pop() !; - const player1 = getLog().pop() !; + const player2 = getLog().pop() !; + const player1 = getLog().pop() !; - expect(player2.keyframes).toEqual([ - {width: PRE_STYLE, offset: 0}, - {width: '0px', offset: 1}, - ]); + expect(player2.keyframes).toEqual([ + {width: PRE_STYLE, offset: 0}, + {width: '0px', offset: 1}, + ]); - expect(player1.keyframes).toEqual([ - {opacity: PRE_STYLE, offset: 0}, {opacity: '0', offset: 1} - ]); + expect(player1.keyframes).toEqual([ + {opacity: PRE_STYLE, offset: 0}, {opacity: '0', offset: 1} + ]); - player2.finish(); - player1.finish(); - assertHasParent(element, false); - }); + player2.finish(); + player1.finish(); + assertHasParent(element, false); + }); it('should properly cancel all existing animations when a removal occurs', () => { @Component({ @@ -3372,43 +3369,42 @@ const DEFAULT_COMPONENT_ID = '1'; expect(getLog().length).toEqual(1); }); - fixmeIvy('FW-951 - Attribute-only synthetic properties are treated differently in Ivy') - .it('should treat the property as true when the expression is missing', () => { - @Component({ - selector: 'parent-cmp', - animations: [ - trigger( - 'myAnimation', - [ - transition( - '* => go', - [ - style({opacity: 0}), - animate(500, style({opacity: 1})), - ]), - ]), - ], - template: ` + it('should treat the property as true when the expression is missing', () => { + @Component({ + selector: 'parent-cmp', + animations: [ + trigger( + 'myAnimation', + [ + transition( + '* => go', + [ + style({opacity: 0}), + animate(500, style({opacity: 1})), + ]), + ]), + ], + template: `
` - }) - class Cmp { - exp = ''; - } + }) + class Cmp { + exp = ''; + } - TestBed.configureTestingModule({declarations: [Cmp]}); + TestBed.configureTestingModule({declarations: [Cmp]}); - const fixture = TestBed.createComponent(Cmp); - const cmp = fixture.componentInstance; - fixture.detectChanges(); - resetLog(); + const fixture = TestBed.createComponent(Cmp); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + resetLog(); - cmp.exp = 'go'; - fixture.detectChanges(); - expect(getLog().length).toEqual(0); - }); + cmp.exp = 'go'; + fixture.detectChanges(); + expect(getLog().length).toEqual(0); + }); it('should respect parent/sub animations when the respective area in the DOM is disabled', fakeAsync(() => {