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 6df627e305..b9b96fcff5 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -3128,6 +3128,159 @@ describe('compiler compliance', () => { expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); }); + it('should not share pure functions between null and object literals', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: \` +
+
+ \` + }) + export class MyApp {} + + @NgModule({declarations: [MyApp]}) + export class MyModule {} + ` + } + }; + + const MyAppDeclaration = ` + const $c0$ = function () { return { foo: null }; }; + const $c1$ = function () { return {}; }; + const $c2$ = function (a0) { return { foo: a0 }; }; + … + MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ + type: MyApp, + selectors: [["ng-component"]], + decls: 2, + vars: 6, + consts: [[${AttributeMarker.Bindings}, "dir"]], + template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "div", 0); + $r3$.ɵɵelement(1, "div", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction0(2, $c0$)); + $r3$.ɵɵadvance(1); + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction1(4, $c2$, $r3$.ɵɵpureFunction0(3, $c1$))); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); + }); + + it('should not share pure functions between null and array literals', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: \` +
+
+ \` + }) + export class MyApp {} + + @NgModule({declarations: [MyApp]}) + export class MyModule {} + ` + } + }; + + const MyAppDeclaration = ` + const $c0$ = function () { return { foo: null }; }; + const $c1$ = function () { return []; }; + const $c2$ = function (a0) { return { foo: a0 }; }; + … + MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ + type: MyApp, + selectors: [["ng-component"]], + decls: 2, + vars: 6, + consts: [[${AttributeMarker.Bindings}, "dir"]], + template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "div", 0); + $r3$.ɵɵelement(1, "div", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction0(2, $c0$)); + $r3$.ɵɵadvance(1); + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction1(4, $c2$, $r3$.ɵɵpureFunction0(3, $c1$))); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); + }); + + it('should not share pure functions between null and function calls', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: \` +
+
+ \` + }) + export class MyApp { + getFoo() { + return 'foo!'; + } + } + + @NgModule({declarations: [MyApp]}) + export class MyModule {} + ` + } + }; + + const MyAppDeclaration = ` + const $c0$ = function () { return { foo: null }; }; + const $c1$ = function (a0) { return { foo: a0 }; }; + … + MyApp.ɵcmp = $r3$.ɵɵdefineComponent({ + type: MyApp, + selectors: [["ng-component"]], + decls: 2, + vars: 5, + consts: [[${AttributeMarker.Bindings}, "dir"]], + template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "div", 0); + $r3$.ɵɵelement(1, "div", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction0(2, $c0$)); + $r3$.ɵɵadvance(1); + $r3$.ɵɵproperty("dir", $r3$.ɵɵpureFunction1(3, $c1$, ctx.getFoo())); + } + }, + encapsulation: 2 + }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, MyAppDeclaration, 'Invalid component definition'); + }); + }); describe('inherited base classes', () => { diff --git a/packages/compiler/src/constant_pool.ts b/packages/compiler/src/constant_pool.ts index 129b39ac01..7be8ff12a2 100644 --- a/packages/compiler/src/constant_pool.ts +++ b/packages/compiler/src/constant_pool.ts @@ -11,6 +11,16 @@ import {OutputContext, error} from './util'; const CONSTANT_PREFIX = '_c'; +/** + * `ConstantPool` tries to reuse literal factories when two or more literals are identical. + * We determine whether literals are identical by creating a key out of their AST using the + * `KeyVisitor`. This constant is used to replace dynamic expressions which can't be safely + * converted into a key. E.g. given an expression `{foo: bar()}`, since we don't know what + * the result of `bar` will be, we create a key that looks like `{foo: }`. Note + * that we use a variable, rather than something like `null` in order to avoid collisions. + */ +const UNKNOWN_VALUE_KEY = o.variable(''); + export const enum DefinitionKind {Injector, Directive, Component, Pipe} /** @@ -127,16 +137,16 @@ export class ConstantPool { getLiteralFactory(literal: o.LiteralArrayExpr|o.LiteralMapExpr): {literalFactory: o.Expression, literalFactoryArguments: o.Expression[]} { - // Create a pure function that builds an array of a mix of constant and variable expressions + // Create a pure function that builds an array of a mix of constant and variable expressions if (literal instanceof o.LiteralArrayExpr) { - const argumentsForKey = literal.entries.map(e => e.isConstant() ? e : o.literal(null)); + const argumentsForKey = literal.entries.map(e => e.isConstant() ? e : UNKNOWN_VALUE_KEY); const key = this.keyOf(o.literalArr(argumentsForKey)); return this._getLiteralFactory(key, literal.entries, entries => o.literalArr(entries)); } else { const expressionForKey = o.literalMap( literal.entries.map(e => ({ key: e.key, - value: e.value.isConstant() ? e.value : o.literal(null), + value: e.value.isConstant() ? e.value : UNKNOWN_VALUE_KEY, quoted: e.quoted }))); const key = this.keyOf(expressionForKey); diff --git a/packages/core/test/acceptance/pure_function_spec.ts b/packages/core/test/acceptance/pure_function_spec.ts index 517b34869b..8b03174a16 100644 --- a/packages/core/test/acceptance/pure_function_spec.ts +++ b/packages/core/test/acceptance/pure_function_spec.ts @@ -559,5 +559,65 @@ describe('components using pure function instructions internally', () => { .not.toBe(secondFixture.componentInstance.directive.value); }); + it('should not confuse object literals and null inside of a literal', () => { + @Component({ + template: ` +
+
+ ` + }) + class App { + @ViewChildren(Dir) directives !: QueryList; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + const values = fixture.componentInstance.directives.map(directive => directive.value); + + expect(values).toEqual([{foo: null}, {foo: {}}]); + }); + + it('should not confuse array literals and null inside of a literal', () => { + @Component({ + template: ` +
+
+ ` + }) + class App { + @ViewChildren(Dir) directives !: QueryList; + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + const values = fixture.componentInstance.directives.map(directive => directive.value); + + expect(values).toEqual([{foo: null}, {foo: []}]); + }); + + it('should not confuse function calls and null inside of a literal', () => { + @Component({ + template: ` +
+
+ ` + }) + class App { + @ViewChildren(Dir) directives !: QueryList; + getFoo() { return 'foo!'; } + } + + TestBed.configureTestingModule({declarations: [Dir, App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + const values = fixture.componentInstance.directives.map(directive => directive.value); + + expect(values).toEqual([{foo: null}, {foo: 'foo!'}]); + }); + + + }); });