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 f2c3344fbf..9c54be9169 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -365,10 +365,10 @@ describe('compiler compliance', () => { template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelement(0, "div", 0); - $r3$.ɵɵpipe(1,"pipe"); + $r3$.ɵɵpipe(1, "pipe"); } if (rf & 2) { - $r3$.ɵɵproperty("ternary", ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $c1$)("pipe", $r3$.ɵɵpipeBind3(1, 4, ctx.value, 1, 2))("and", ctx.cond && $r3$.ɵɵpureFunction1(10, $c0$, ctx.b))("or", ctx.cond || $r3$.ɵɵpureFunction1(12, $c0$, ctx.c)); + $r3$.ɵɵproperty("ternary", ctx.cond ? $r3$.ɵɵpureFunction1(8, $c0$, ctx.a): $r3$.ɵɵpureFunction0(10, $c1$))("pipe", $r3$.ɵɵpipeBind3(1, 4, ctx.value, 1, 2))("and", ctx.cond && $r3$.ɵɵpureFunction1(11, $c0$, ctx.b))("or", ctx.cond || $r3$.ɵɵpureFunction1(13, $c0$, ctx.c)); } } `; @@ -1082,25 +1082,24 @@ describe('compiler compliance', () => { }; const MyAppDefinition = ` - const $c0$ = {opacity: 0, duration: 0}; + const $c0$ = function () { return {opacity: 0, duration: 0}; }; const $e0_ff$ = function ($v$) { return {opacity: 1, duration: $v$}; }; - const $e0_ff_1$ = function ($v$) { return [$c0$, $v$]; }; + const $e0_ff_1$ = function ($v1$, $v2$) { return [$v1$, $v2$]; }; const $e0_ff_2$ = function ($v1$, $v2$) { return {animation: $v1$, actions: $v2$}; }; … MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ type: MyApp, selectors: [["my-app"]], decls: 1, - vars: 8, + vars: 10, consts: [[${AttributeMarker.Bindings}, "config"]], template: function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵɵelement(0, "nested-comp", 0); } if (rf & 2) { - $r3$.ɵɵproperty( - "config", - $r3$.ɵɵpureFunction2(5, $e0_ff_2$, ctx.name, $r3$.ɵɵpureFunction1(3, $e0_ff_1$, $r3$.ɵɵpureFunction1(1, $e0_ff$, ctx.duration)))); + $r3$.ɵɵproperty("config", + $r3$.ɵɵpureFunction2(7, $e0_ff_2$, ctx.name, $r3$.ɵɵpureFunction2(4, $e0_ff_1$, $r3$.ɵɵpureFunction0(1, $c0$), $r3$.ɵɵpureFunction1(2, $e0_ff$, ctx.duration)))); } }, directives: [NestedComp], @@ -2975,6 +2974,88 @@ describe('compiler compliance', () => { expectEmit(result.source, expectedOutput, 'Invalid directive definition'); }); + it('should generate a pure function for constant object literals', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '' + }) + export class MyApp { + } + ` + } + }; + + const MyAppDeclaration = ` + const $c0$ = function () { return {}; }; + const $c1$ = function () { return { a: 1, b: 2 }; }; + … + MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ + type: MyApp, + selectors: [["ng-component"]], + decls: 1, + vars: 4, + consts: [[${AttributeMarker.Bindings}, "prop", "otherProp"]], + template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "some-comp", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("prop", $r3$.ɵɵpureFunction0(2, $c0$))("otherProp", $r3$.ɵɵpureFunction0(3, $c1$)); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); + }); + + it('should generate a pure function for constant array literals', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '' + }) + export class MyApp { + } + ` + } + }; + + const MyAppDeclaration = ` + const $c0$ = function () { return []; }; + const $c1$ = function () { return [0, 1, 2]; }; + … + MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ + type: MyApp, + selectors: [["ng-component"]], + decls: 1, + vars: 4, + consts: [[${AttributeMarker.Bindings}, "prop", "otherProp"]], + template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "some-comp", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("prop", $r3$.ɵɵpureFunction0(2, $c0$))("otherProp", $r3$.ɵɵpureFunction0(3, $c1$)); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); + }); + }); describe('inherited base classes', () => { 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 a5413bd41f..23fff07a73 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 @@ -915,7 +915,7 @@ describe('compiler compliance: styling', () => { if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 8, $ctx$.myStyleExp, 1000)); - $r3$.ɵɵclassMap($e2_styling$); + $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$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 17, $ctx$.fooExp, 2000)); diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 9eba6ff72f..9b0b5bd9ea 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -1420,11 +1420,9 @@ export class ValueConverter extends AstMemoryEfficientTransformer { array.span, array.sourceSpan, this.visitAll(array.expressions), values => { // If the literal has calculated (non-literal) elements transform it into // calls to literal factories that compose the literal and will cache intermediate - // values. Otherwise, just return an literal array that contains the values. + // values. const literal = o.literalArr(values); - return values.every(a => a.isConstant()) ? - this.constantPool.getConstLiteral(literal, true) : - getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots); + return getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots); }); } @@ -1432,12 +1430,10 @@ export class ValueConverter extends AstMemoryEfficientTransformer { return new BuiltinFunctionCall(map.span, map.sourceSpan, this.visitAll(map.values), values => { // If the literal has calculated (non-literal) elements transform it into // calls to literal factories that compose the literal and will cache intermediate - // values. Otherwise, just return an literal array that contains the values. + // values. const literal = o.literalMap(values.map( (value, index) => ({key: map.keys[index].key, value, quoted: map.keys[index].quoted}))); - return values.every(a => a.isConstant()) ? - this.constantPool.getConstLiteral(literal, true) : - getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots); + return getLiteralFactory(this.constantPool, literal, this.allocatePureFunctionSlots); }); } } @@ -1484,15 +1480,11 @@ function getLiteralFactory( const {literalFactory, literalFactoryArguments} = constantPool.getLiteralFactory(literal); // Allocate 1 slot for the result plus 1 per argument const startSlot = allocateSlots(1 + literalFactoryArguments.length); - literalFactoryArguments.length > 0 || error(`Expected arguments to a literal factory function`); const {identifier, isVarLength} = pureFunctionCallInfo(literalFactoryArguments); // Literal factories are pure functions that only need to be re-invoked when the parameters // change. - const args = [ - o.literal(startSlot), - literalFactory, - ]; + const args = [o.literal(startSlot), literalFactory]; if (isVarLength) { args.push(o.literalArr(literalFactoryArguments)); diff --git a/packages/core/src/render3/instructions/container.ts b/packages/core/src/render3/instructions/container.ts index d481f8ebe2..40e6a50bf6 100644 --- a/packages/core/src/render3/instructions/container.ts +++ b/packages/core/src/render3/instructions/container.ts @@ -73,8 +73,8 @@ export function ɵɵtemplate( // TODO: consider a separate node type for templates const tContainerNode = containerInternal( - lView, index, tagName || null, getConstant(tViewConsts, attrsIndex) as TAttributes); - const localRefs = getConstant(tViewConsts, localRefsIndex) as string[]; + lView, index, tagName || null, getConstant(tViewConsts, attrsIndex)); + const localRefs = getConstant(tViewConsts, localRefsIndex); if (tView.firstCreatePass) { ngDevMode && ngDevMode.firstCreatePass++; resolveDirectives(tView, lView, tContainerNode, localRefs); diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 14b31cea00..4dda458a90 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -46,8 +46,8 @@ export function ɵɵelementStart( const lView = getLView(); const tView = lView[TVIEW]; const tViewConsts = tView.consts; - const attrs = getConstant(tViewConsts, attrsIndex) as TAttributes; - const localRefs = getConstant(tViewConsts, localRefsIndex) as string[]; + const attrs = getConstant(tViewConsts, attrsIndex); + const localRefs = getConstant(tViewConsts, localRefsIndex); ngDevMode && assertEqual( getBindingIndex(), tView.bindingStartIndex, 'elements should be created before any bindings'); diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts index 47565f7c51..4752b22097 100644 --- a/packages/core/src/render3/instructions/element_container.ts +++ b/packages/core/src/render3/instructions/element_container.ts @@ -43,8 +43,8 @@ export function ɵɵelementContainerStart( const renderer = lView[RENDERER]; const tagName = 'ng-container'; const tViewConsts = tView.consts; - const attrs = getConstant(tViewConsts, attrsIndex) as TAttributes; - const localRefs = getConstant(tViewConsts, localRefsIndex) as string[]; + const attrs = getConstant(tViewConsts, attrsIndex); + const localRefs = getConstant(tViewConsts, localRefsIndex); ngDevMode && assertEqual( getBindingIndex(), tView.bindingStartIndex, 'element containers should be created before any bindings'); diff --git a/packages/core/src/render3/pure_function.ts b/packages/core/src/render3/pure_function.ts index d0c5cc59ac..980e0620bb 100644 --- a/packages/core/src/render3/pure_function.ts +++ b/packages/core/src/render3/pure_function.ts @@ -8,7 +8,7 @@ import {bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, getBinding, updateBinding} from './bindings'; import {getBindingRoot, getLView} from './state'; -import {isCreationMode} from './util/view_utils'; +import {NO_CHANGE} from './tokens'; /** @@ -44,7 +44,7 @@ export function ɵɵpureFunction0(slotOffset: number, pureFn: () => T, thisAr // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings const bindingIndex = getBindingRoot() + slotOffset; const lView = getLView(); - return isCreationMode(lView) ? + return lView[bindingIndex] === NO_CHANGE ? updateBinding(lView, bindingIndex, thisArg ? pureFn.call(thisArg) : pureFn()) : getBinding(lView, bindingIndex); } diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index c9250b25f5..e03fdd5910 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -176,8 +176,9 @@ export function viewAttachedToContainer(view: LView): boolean { } /** Returns a constant from `TConstants` instance. */ -export function getConstant(consts: TConstants | null, index: number | null | undefined) { - return consts === null || index == null ? null : consts[index]; +export function getConstant(consts: TConstants | null, index: number | null | undefined): T| + null { + return consts === null || index == null ? null : consts[index] as unknown as T; } /** diff --git a/packages/core/test/acceptance/pure_function_spec.ts b/packages/core/test/acceptance/pure_function_spec.ts index 01abb54b2e..517b34869b 100644 --- a/packages/core/test/acceptance/pure_function_spec.ts +++ b/packages/core/test/acceptance/pure_function_spec.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ import {CommonModule} from '@angular/common'; -import {Component, Input} from '@angular/core'; +import {Component, Directive, Input, QueryList, ViewChild, ViewChildren} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; +import {onlyInIvy} from '@angular/private/testing'; describe('components using pure function instructions internally', () => { describe('with array literals', () => { @@ -479,4 +480,84 @@ describe('components using pure function instructions internally', () => { expect(objectComps[1].config).toEqual({opacity: 1, duration: 600}); }); }); + + onlyInIvy('issue has only been fixed for Ivy').describe('identical literals', () => { + @Directive({selector: '[dir]'}) + class Dir { + @Input('dir') value: any; + } + + it('should not share object literals across elements', () => { + @Component({ + template: ` +
+
+ ` + }) + class App { + @ViewChildren(Dir) directives !: QueryList; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const directives = fixture.componentInstance.directives.toArray(); + expect(directives[0].value).not.toBe(directives[1].value); + }); + + it('should not share array literals across elements', () => { + @Component({ + template: ` +
+
+ ` + }) + class App { + @ViewChildren(Dir) directives !: QueryList; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const directives = fixture.componentInstance.directives.toArray(); + expect(directives[0].value).not.toBe(directives[1].value); + }); + + it('should not share object literals across component instances', () => { + @Component({template: `
`}) + class App { + @ViewChild(Dir) directive !: Dir; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const firstFixture = TestBed.createComponent(App); + firstFixture.detectChanges(); + + const secondFixture = TestBed.createComponent(App); + secondFixture.detectChanges(); + + expect(firstFixture.componentInstance.directive.value) + .not.toBe(secondFixture.componentInstance.directive.value); + }); + + it('should not share array literals across component instances', () => { + @Component({template: `
`}) + class App { + @ViewChild(Dir) directive !: Dir; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const firstFixture = TestBed.createComponent(App); + firstFixture.detectChanges(); + + const secondFixture = TestBed.createComponent(App); + secondFixture.detectChanges(); + + expect(firstFixture.componentInstance.directive.value) + .not.toBe(secondFixture.componentInstance.directive.value); + }); + + }); });