From 0d516f16585bc7e9cf716a2bd5bc44110e23dc5f Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 10 Apr 2018 20:57:20 -0700 Subject: [PATCH] fix(ivy): update compiler to generate separate creation mode and update mode blocks (#23292) PR Close #23292 --- .../src/transformers/node_emitter.ts | 13 +- packages/compiler/design/architecture.md | 8 +- .../compiler/src/output/abstract_emitter.ts | 7 +- packages/compiler/src/output/output_ast.ts | 7 +- .../compiler/src/render3/r3_view_compiler.ts | 61 ++++--- .../render3/r3_compiler_compliance_spec.ts | 168 ++++++++++-------- .../render3/r3_view_compiler_i18n_spec.ts | 12 +- .../render3/r3_view_compiler_listener_spec.ts | 6 +- .../render3/r3_view_compiler_template_spec.ts | 45 +++-- packages/core/src/render3/instructions.ts | 69 ++++--- packages/core/src/render3/interfaces/view.ts | 11 +- packages/core/test/bundling/todo/index.ts | 16 +- .../test/render3/common_integration_spec.ts | 63 ++++++- packages/core/test/render3/common_with_def.ts | 10 +- 14 files changed, 328 insertions(+), 168 deletions(-) diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index 62959fad61..7981b3b01a 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -566,12 +566,16 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { /* type */ undefined, this._visitStatements(expr.statements))); } - visitBinaryOperatorExpr(expr: BinaryOperatorExpr): RecordedNode { + visitBinaryOperatorExpr(expr: BinaryOperatorExpr): + RecordedNode { let binaryOperator: ts.BinaryOperator; switch (expr.operator) { case BinaryOperator.And: binaryOperator = ts.SyntaxKind.AmpersandAmpersandToken; break; + case BinaryOperator.BitwiseAnd: + binaryOperator = ts.SyntaxKind.AmpersandToken; + break; case BinaryOperator.Bigger: binaryOperator = ts.SyntaxKind.GreaterThanToken; break; @@ -617,10 +621,9 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { default: throw new Error(`Unknown operator: ${expr.operator}`); } - return this.record( - expr, ts.createParen(ts.createBinary( - expr.lhs.visitExpression(this, null), binaryOperator, - expr.rhs.visitExpression(this, null)))); + const binary = ts.createBinary( + expr.lhs.visitExpression(this, null), binaryOperator, expr.rhs.visitExpression(this, null)); + return this.record(expr, expr.parens ? ts.createParen(binary) : binary); } visitReadPropExpr(expr: ReadPropExpr): RecordedNode { diff --git a/packages/compiler/design/architecture.md b/packages/compiler/design/architecture.md index ec5fbc66b1..396e0dcfec 100644 --- a/packages/compiler/design/architecture.md +++ b/packages/compiler/design/architecture.md @@ -85,13 +85,15 @@ GreetComponent.ngComponentDef = i0.ɵdefineComponent({ type: GreetComponent, tag: 'greet', factory: () => new GreetComponent(), - template: function (ctx, cm) { - if (cm) { + template: function (rf, ctx) { + if (rf & RenderFlags.Create) { i0.ɵE(0, 'div'); i0.ɵT(1); i0.ɵe(); } - i0.ɵt(1, i0.ɵi1('Hello ', ctx.name, '!')); + if (rf & RenderFlags.Update) { + i0.ɵt(1, i0.ɵi1('Hello ', ctx.name, '!')); + } } }); ``` diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index 3909393c8a..8a49a96aa5 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -396,6 +396,9 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex case o.BinaryOperator.And: opStr = '&&'; break; + case o.BinaryOperator.BitwiseAnd: + opStr = '&'; + break; case o.BinaryOperator.Or: opStr = '||'; break; @@ -429,11 +432,11 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex default: throw new Error(`Unknown operator ${ast.operator}`); } - ctx.print(ast, `(`); + if (ast.parens) ctx.print(ast, `(`); ast.lhs.visitExpression(this, ctx); ctx.print(ast, ` ${opStr} `); ast.rhs.visitExpression(this, ctx); - ctx.print(ast, `)`); + if (ast.parens) ctx.print(ast, `)`); return null; } diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 246adf27c8..69e0ac2f5d 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -99,6 +99,7 @@ export enum BinaryOperator { Modulo, And, Or, + BitwiseAnd, Lower, LowerEquals, Bigger, @@ -207,6 +208,10 @@ export abstract class Expression { and(rhs: Expression, sourceSpan?: ParseSourceSpan|null): BinaryOperatorExpr { return new BinaryOperatorExpr(BinaryOperator.And, this, rhs, null, sourceSpan); } + bitwiseAnd(rhs: Expression, sourceSpan?: ParseSourceSpan|null, parens: boolean = true): + BinaryOperatorExpr { + return new BinaryOperatorExpr(BinaryOperator.BitwiseAnd, this, rhs, null, sourceSpan, parens); + } or(rhs: Expression, sourceSpan?: ParseSourceSpan|null): BinaryOperatorExpr { return new BinaryOperatorExpr(BinaryOperator.Or, this, rhs, null, sourceSpan); } @@ -569,7 +574,7 @@ export class BinaryOperatorExpr extends Expression { public lhs: Expression; constructor( public operator: BinaryOperator, lhs: Expression, public rhs: Expression, type?: Type|null, - sourceSpan?: ParseSourceSpan|null) { + sourceSpan?: ParseSourceSpan|null, public parens: boolean = true) { super(type || lhs.type, sourceSpan); this.lhs = lhs; } diff --git a/packages/compiler/src/render3/r3_view_compiler.ts b/packages/compiler/src/render3/r3_view_compiler.ts index 5221b6ea83..18db0732fa 100644 --- a/packages/compiler/src/render3/r3_view_compiler.ts +++ b/packages/compiler/src/render3/r3_view_compiler.ts @@ -26,8 +26,8 @@ import {BUILD_OPTIMIZER_COLOCATE, OutputMode} from './r3_types'; /** Name of the context parameter passed into a template function */ const CONTEXT_NAME = 'ctx'; -/** Name of the creation mode flag passed into a template function */ -const CREATION_MODE_FLAG = 'cm'; +/** Name of the RenderFlag passed into a template function */ +const RENDER_FLAGS = 'rf'; /** Name of the temporary to use during data binding */ const TEMPORARY_NAME = '_t'; @@ -421,15 +421,31 @@ class BindingScope implements LocalResolver { } } +// Pasted from render3/interfaces/definition since it cannot be referenced directly +/** + * Flags passed into template functions to determine which blocks (i.e. creation, update) + * should be executed. + * + * Typically, a template runs both the creation block and the update block on initialization and + * subsequent runs only execute the update block. However, dynamically created views require that + * the creation block be executed separately from the update block (for backwards compat). + */ +export const enum RenderFlags { + /* Whether to run the creation block (e.g. create elements and directives) */ + Create = 0b01, + + /* Whether to run the update block (e.g. refresh bindings) */ + Update = 0b10 +} + class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { private _dataIndex = 0; private _bindingContext = 0; - private _referenceIndex = 0; private _temporaryAllocated = false; private _prefix: o.Statement[] = []; private _creationMode: o.Statement[] = []; + private _variableMode: o.Statement[] = []; private _bindingMode: o.Statement[] = []; - private _refreshMode: o.Statement[] = []; private _postfix: o.Statement[] = []; private _contentProjections: Map; private _projectionDefinitionIndex = 0; @@ -530,7 +546,15 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { templateVisitAll(this, asts); const creationMode = this._creationMode.length > 0 ? - [o.ifStmt(o.variable(CREATION_MODE_FLAG), this._creationMode)] : + [o.ifStmt( + o.variable(RENDER_FLAGS).bitwiseAnd(o.literal(RenderFlags.Create), null, false), + this._creationMode)] : + []; + + const updateMode = this._bindingMode.length > 0 ? + [o.ifStmt( + o.variable(RENDER_FLAGS).bitwiseAnd(o.literal(RenderFlags.Update), null, false), + this._bindingMode)] : []; // Generate maps of placeholder name to node indexes @@ -547,18 +571,16 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { } return o.fn( + [new o.FnParam(RENDER_FLAGS, o.NUMBER_TYPE), new o.FnParam(this.contextParameter, null)], [ - new o.FnParam(this.contextParameter, null), new o.FnParam(CREATION_MODE_FLAG, o.BOOL_TYPE) - ], - [ - // Temporary variable declarations (i.e. let _t: any;) + // Temporary variable declarations for query refresh (i.e. let _t: any;) ...this._prefix, - // Creating mode (i.e. if (cm) { ... }) + // Creating mode (i.e. if (rf & RenderFlags.Create) { ... }) ...creationMode, - // Binding mode (i.e. ɵp(...)) - ...this._bindingMode, - // Refresh mode (i.e. Comp.r(...)) - ...this._refreshMode, + // Temporary variable declarations for local refs (i.e. const tmp = ld(1) as any) + ...this._variableMode, + // Binding and refresh mode (i.e. if (rf & RenderFlags.Update) {...}) + ...updateMode, // Nested templates (i.e. function CompTemplate() {}) ...this._postfix ], @@ -665,9 +687,9 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { referenceDataSlots.set(reference.name, slot); // Generate the update temporary. const variableName = this.bindingScope.freshReferenceName(); - this._bindingMode.push(o.variable(variableName, o.INFERRED_TYPE) - .set(o.importExpr(R3.load).callFn([o.literal(slot)])) - .toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final])); + this._variableMode.push(o.variable(variableName, o.INFERRED_TYPE) + .set(o.importExpr(R3.load).callFn([o.literal(slot)])) + .toDeclStmt(o.INFERRED_TYPE, [o.StmtModifier.Final])); this.bindingScope.set(reference.name, o.variable(variableName)); return [reference.name, reference.originalValue]; })).map(value => o.literal(value)); @@ -836,9 +858,8 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { // Creation mode this.instruction(this._creationMode, ast.sourceSpan, R3.text, o.literal(nodeIndex)); - // Refresh mode this.instruction( - this._refreshMode, ast.sourceSpan, R3.textCreateBound, o.literal(nodeIndex), + this._bindingMode, ast.sourceSpan, R3.textCreateBound, o.literal(nodeIndex), this.convertPropertyBinding(o.variable(CONTEXT_NAME), ast.value)); } @@ -899,7 +920,7 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { const convertedPropertyBinding = convertPropertyBinding( this, implicit, pipesConvertedValue, this.bindingContext(), BindingForm.TrySimple, interpolate); - this._refreshMode.push(...convertedPropertyBinding.stmts); + this._bindingMode.push(...convertedPropertyBinding.stmts); return convertedPropertyBinding.currValExpr; } } diff --git a/packages/compiler/test/render3/r3_compiler_compliance_spec.ts b/packages/compiler/test/render3/r3_compiler_compliance_spec.ts index b006679009..6c96915232 100644 --- a/packages/compiler/test/render3/r3_compiler_compliance_spec.ts +++ b/packages/compiler/test/render3/r3_compiler_compliance_spec.ts @@ -13,6 +13,7 @@ import {compile, expectEmit} from './mock_compile'; * test in compiler_canonical_spec.ts should have a corresponding test here. */ describe('compiler compliance', () => { + const angularFiles = setup({ compileAngular: true, compileAnimations: false, @@ -45,8 +46,8 @@ describe('compiler compliance', () => { const template = ` const $c1$ = ['class', 'my-app', 'title', 'Hello']; … - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'div', $c1$); $r3$.ɵT(1, 'Hello '); $r3$.ɵE(2, 'b'); @@ -94,8 +95,8 @@ describe('compiler compliance', () => { type: ChildComponent, selectors: [['child']], factory: function ChildComponent_Factory() { return new ChildComponent(); }, - template: function ChildComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function ChildComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵT(0, 'child-view'); } } @@ -118,8 +119,8 @@ describe('compiler compliance', () => { type: MyComponent, selectors: [['my-component']], factory: function MyComponent_Factory() { return new MyComponent(); }, - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'child', $c1$); $r3$.ɵe(); $r3$.ɵT(1, '!'); @@ -255,21 +256,22 @@ describe('compiler compliance', () => { type: MyComponent, selectors: [['my-component']], factory: function MyComponent_Factory() { return new MyComponent(); }, - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'ul', null, $c1$); $r3$.ɵC(2, MyComponent_IfDirective_Template_2, null, $c2$); $r3$.ɵe(); } const $foo$ = $r3$.ɵld(1); - - function MyComponent_IfDirective_Template_2(ctx0: IDENT, cm: IDENT) { - if (cm) { + function MyComponent_IfDirective_Template_2(rf: IDENT, ctx0: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'li'); $r3$.ɵT(1); $r3$.ɵe(); } - $r3$.ɵt(1, $r3$.ɵi2('', ctx.salutation, ' ', $foo$, '')); + if (rf & 2) { + $r3$.ɵt(1, $r3$.ɵi2('', ctx.salutation, ' ', $foo$, '')); + } } }, directives: [IfDirective] @@ -324,12 +326,14 @@ describe('compiler compliance', () => { type: MyApp, selectors: [['my-app']], factory: function MyApp_Factory() { return new MyApp(); }, - template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) { - if (cm) { + template: function MyApp_Template(rf: $RenderFlags$, ctx: $MyApp$) { + if (rf & 1) { $r3$.ɵE(0, 'my-comp'); $r3$.ɵe(); } - $r3$.ɵp(0, 'names', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.customName))); + if (rf & 2) { + $r3$.ɵp(0, 'names', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.customName))); + } }, directives: [MyComp] }); @@ -401,14 +405,16 @@ describe('compiler compliance', () => { type: MyApp, selectors: [['my-app']], factory: function MyApp_Factory() { return new MyApp(); }, - template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) { - if (cm) { + template: function MyApp_Template(rf: $RenderFlags$, ctx: $MyApp$) { + if (rf & 1) { $r3$.ɵE(0, 'my-comp'); $r3$.ɵe(); } - $r3$.ɵp( - 0, 'names', - $r3$.ɵb($r3$.ɵfV($e0_ff$, ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8))); + if (rf & 2) { + $r3$.ɵp( + 0, 'names', + $r3$.ɵb($r3$.ɵfV($e0_ff$, ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8))); + } }, directives: [MyComp] }); @@ -460,12 +466,14 @@ describe('compiler compliance', () => { type: MyApp, selectors: [['my-app']], factory: function MyApp_Factory() { return new MyApp(); }, - template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) { - if (cm) { + template: function MyApp_Template(rf: $RenderFlags$, ctx: $MyApp$) { + if (rf & 1) { $r3$.ɵE(0, 'object-comp'); $r3$.ɵe(); } - $r3$.ɵp(0, 'config', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.name))); + if (rf & 2) { + $r3$.ɵp(0, 'config', $r3$.ɵb($r3$.ɵf1($e0_ff$, ctx.name))); + } }, directives: [ObjectComp] }); @@ -523,15 +531,17 @@ describe('compiler compliance', () => { type: MyApp, selectors: [['my-app']], factory: function MyApp_Factory() { return new MyApp(); }, - template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) { - if (cm) { + template: function MyApp_Template(rf: $RenderFlags$, ctx: $MyApp$) { + if (rf & 1) { $r3$.ɵE(0, 'nested-comp'); $r3$.ɵe(); } - $r3$.ɵp( - 0, 'config', - $r3$.ɵb($r3$.ɵf2( - $e0_ff_2$, ctx.name, $r3$.ɵf1($e0_ff_1$, $r3$.ɵf1($e0_ff$, ctx.duration))))); + if (rf & 2) { + $r3$.ɵp( + 0, 'config', + $r3$.ɵb($r3$.ɵf2( + $e0_ff_2$, ctx.name, $r3$.ɵf1($e0_ff_1$, $r3$.ɵf1($e0_ff$, ctx.duration))))); + } }, directives: [NestedComp] }); @@ -579,8 +589,8 @@ describe('compiler compliance', () => { type: SimpleComponent, selectors: [['simple']], factory: function SimpleComponent_Factory() { return new SimpleComponent(); }, - template: function SimpleComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function SimpleComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵpD(0); $r3$.ɵE(1, 'div'); $r3$.ɵP(2, 0); @@ -598,8 +608,8 @@ describe('compiler compliance', () => { type: ComplexComponent, selectors: [['complex']], factory: function ComplexComponent_Factory() { return new ComplexComponent(); }, - template: function ComplexComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function ComplexComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵpD(0, $c1$); $r3$.ɵE(1, 'div', $c2$); $r3$.ɵP(2, 0, 1); @@ -663,14 +673,16 @@ describe('compiler compliance', () => { type: ViewQueryComponent, selectors: [['view-query-component']], factory: function ViewQueryComponent_Factory() { return new ViewQueryComponent(); }, - template: function ViewQueryComponent_Template(ctx: $ViewQueryComponent$, cm: $boolean$) { + template: function ViewQueryComponent_Template(rf: $RenderFlags$, ctx: $ViewQueryComponent$) { var $tmp$: $any$; - if (cm) { + if (rf & 1) { $r3$.ɵQ(0, SomeDirective, true); $r3$.ɵE(1, 'div', $e0_attrs$); $r3$.ɵe(); } - ($r3$.ɵqR(($tmp$ = $r3$.ɵld(0))) && (ctx.someDir = $tmp$.first)); + if (rf & 2) { + ($r3$.ɵqR(($tmp$ = $r3$.ɵld(0))) && (ctx.someDir = $tmp$.first)); + } }, directives:[SomeDirective] });`; @@ -728,8 +740,8 @@ describe('compiler compliance', () => { ($r3$.ɵqR(($tmp$ = $r3$.ɵld(dirIndex)[1])) && ($r3$.ɵld(dirIndex)[0].someDir = $tmp$.first)); }, template: function ContentQueryComponent_Template( - ctx: $ContentQueryComponent$, cm: $boolean$) { - if (cm) { + rf: $RenderFlags$, ctx: $ContentQueryComponent$) { + if (rf & 1) { $r3$.ɵpD(0); $r3$.ɵE(1, 'div'); $r3$.ɵP(2, 0); @@ -805,8 +817,8 @@ describe('compiler compliance', () => { type: MyApp, selectors: [['my-app']], factory: function MyApp_Factory() { return new MyApp(); }, - template: function MyApp_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyApp_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵT(0); $r3$.ɵPp(1, 'myPurePipe'); $r3$.ɵPp(2, 'myPipe'); @@ -815,8 +827,10 @@ describe('compiler compliance', () => { $r3$.ɵPp(5, 'myPurePipe'); $r3$.ɵe(); } - $r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, $r3$.ɵpb2(2,ctx.name, ctx.size), ctx.size), '')); - $r3$.ɵt(4, $r3$.ɵi1('', $r3$.ɵpb2(5, ctx.name, ctx.size), '')); + if (rf & 2) { + $r3$.ɵt(0, $r3$.ɵi1('', $r3$.ɵpb2(1, $r3$.ɵpb2(2,ctx.name, ctx.size), ctx.size), '')); + $r3$.ɵt(4, $r3$.ɵi1('', $r3$.ɵpb2(5, ctx.name, ctx.size), '')); + } }, pipes: [MyPurePipe, MyPipe] });`; @@ -852,14 +866,16 @@ describe('compiler compliance', () => { type: MyComponent, selectors: [['my-component']], factory: function MyComponent_Factory() { return new MyComponent(); }, - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'input', null, $c1$); $r3$.ɵe(); $r3$.ɵT(2); } const $user$ = $r3$.ɵld(1); - $r3$.ɵt(2, $r3$.ɵi1('Hello ', $user$.value, '!')); + if (rf & 2) { + $r3$.ɵt(2, $r3$.ɵi1('Hello ', $user$.value, '!')); + } } }); `; @@ -920,7 +936,7 @@ describe('compiler compliance', () => { type: LifecycleComp, selectors: [['lifecycle-comp']], factory: function LifecycleComp_Factory() { return new LifecycleComp(); }, - template: function LifecycleComp_Template(ctx: IDENT, cm: IDENT) {}, + template: function LifecycleComp_Template(rf: IDENT, ctx: IDENT) {}, inputs: {nameMin: 'name'}, features: [$r3$.ɵNgOnChangesFeature(LifecycleComp)] });`; @@ -930,15 +946,17 @@ describe('compiler compliance', () => { type: SimpleLayout, selectors: [['simple-layout']], factory: function SimpleLayout_Factory() { return new SimpleLayout(); }, - template: function SimpleLayout_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function SimpleLayout_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'lifecycle-comp'); $r3$.ɵe(); $r3$.ɵE(1, 'lifecycle-comp'); $r3$.ɵe(); } - $r3$.ɵp(0, 'name', $r3$.ɵb(ctx.name1)); - $r3$.ɵp(1, 'name', $r3$.ɵb(ctx.name2)); + if (rf & 2) { + $r3$.ɵp(0, 'name', $r3$.ɵb(ctx.name1)); + $r3$.ɵp(1, 'name', $r3$.ɵb(ctx.name2)); + } }, directives: [LifecycleComp] });`; @@ -1049,22 +1067,26 @@ describe('compiler compliance', () => { type: MyComponent, selectors: [['my-component']], factory: function MyComponent_Factory() { return new MyComponent(); }, - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'ul'); $r3$.ɵC(1, MyComponent_ForOfDirective_Template_1, null, $_c0$); $r3$.ɵe(); } - $r3$.ɵp(1, 'forOf', $r3$.ɵb(ctx.items)); + if (rf & 2) { + $r3$.ɵp(1, 'forOf', $r3$.ɵb(ctx.items)); + } - function MyComponent_ForOfDirective_Template_1(ctx0: IDENT, cm: IDENT) { - if (cm) { + function MyComponent_ForOfDirective_Template_1(rf: IDENT, ctx0: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'li'); $r3$.ɵT(1); $r3$.ɵe(); } - const $item$ = ctx0.$implicit; - $r3$.ɵt(1, $r3$.ɵi1('', $item$.name, '')); + if (rf & 2) { + const $item$ = ctx0.$implicit; + $r3$.ɵt(1, $r3$.ɵi1('', $item$.name, '')); + } } }, directives: [ForOfDirective] @@ -1123,16 +1145,18 @@ describe('compiler compliance', () => { type: MyComponent, selectors: [['my-component']], factory: function MyComponent_Factory() { return new MyComponent(); }, - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'ul'); $r3$.ɵC(1, MyComponent_ForOfDirective_Template_1, null, $c1$); $r3$.ɵe(); } - $r3$.ɵp(1, 'forOf', $r3$.ɵb(ctx.items)); + if (rf & 2) { + $r3$.ɵp(1, 'forOf', $r3$.ɵb(ctx.items)); + } - function MyComponent_ForOfDirective_Template_1(ctx0: IDENT, cm: IDENT) { - if (cm) { + function MyComponent_ForOfDirective_Template_1(rf: IDENT, ctx0: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'li'); $r3$.ɵE(1, 'div'); $r3$.ɵT(2); @@ -1142,20 +1166,24 @@ describe('compiler compliance', () => { $r3$.ɵe(); $r3$.ɵe(); } - const $item$ = ctx0.$implicit; - $r3$.ɵp(4, 'forOf', $r3$.ɵb(IDENT.infos)); - $r3$.ɵt(2, $r3$.ɵi1('', IDENT.name, '')); + if (rf & 2) { + const $item$ = ctx0.$implicit; + $r3$.ɵt(2, $r3$.ɵi1('', IDENT.name, '')); + $r3$.ɵp(4, 'forOf', $r3$.ɵb(IDENT.infos)); + } function MyComponent_ForOfDirective_ForOfDirective_Template_4( - ctx1: IDENT, cm: IDENT) { - if (cm) { + rf: IDENT, ctx1: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'li'); $r3$.ɵT(1); $r3$.ɵe(); } - const $item$ = ctx0.$implicit; - const $info$ = ctx1.$implicit; - $r3$.ɵt(1, $r3$.ɵi2(' ', $item$.name, ': ', $info$.description, ' ')); + if (rf & 2) { + const $item$ = ctx0.$implicit; + const $info$ = ctx1.$implicit; + $r3$.ɵt(1, $r3$.ɵi2(' ', $item$.name, ': ', $info$.description, ' ')); + } } } }, diff --git a/packages/compiler/test/render3/r3_view_compiler_i18n_spec.ts b/packages/compiler/test/render3/r3_view_compiler_i18n_spec.ts index e11d8fd5f1..d11cacc4c2 100644 --- a/packages/compiler/test/render3/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler/test/render3/r3_view_compiler_i18n_spec.ts @@ -46,8 +46,8 @@ describe('i18n support in the view compiler', () => { const $msg_1$ = goog.getMsg('Hello world'); const $msg_2$ = goog.getMsg('farewell'); … - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { … $r3$.ɵT(1, $msg_1$); … @@ -104,8 +104,8 @@ describe('i18n support in the view compiler', () => { */ const $msg_2$ = goog.getMsg('Hello world'); … - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'div', $r3$.ɵf1($c1$, $msg_1$)); $r3$.ɵT(1, $msg_2$); $r3$.ɵe(); @@ -152,8 +152,8 @@ describe('i18n support in the view compiler', () => { return ['id', 'static', 'title', $a1$]; }; … - template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { - if (cm) { + template: function MyComponent_Template(rf: IDENT, ctx: IDENT) { + if (rf & 1) { $r3$.ɵE(0, 'div', $r3$.ɵf1($c1$, $msg_1$)); $r3$.ɵe(); } diff --git a/packages/compiler/test/render3/r3_view_compiler_listener_spec.ts b/packages/compiler/test/render3/r3_view_compiler_listener_spec.ts index 882b601b5d..a45f7a58b3 100644 --- a/packages/compiler/test/render3/r3_view_compiler_listener_spec.ts +++ b/packages/compiler/test/render3/r3_view_compiler_listener_spec.ts @@ -41,8 +41,8 @@ describe('compiler compliance: listen()', () => { // The template should look like this (where IDENT is a wild card for an identifier): const template = ` - template: function MyComponent_Template(ctx: $MyComponent$, cm: $boolean$) { - if (cm) { + template: function MyComponent_Template(rf: $RenderFlags$, ctx: $MyComponent$) { + if (rf & 1) { $r3$.ɵE(0, 'div'); $r3$.ɵL('click', function MyComponent_Template_div_click_listener($event: $any$) { ctx.onClick($event); @@ -59,4 +59,4 @@ describe('compiler compliance: listen()', () => { expectEmit(result.source, template, 'Incorrect template'); }); -}); \ No newline at end of file +}); diff --git a/packages/compiler/test/render3/r3_view_compiler_template_spec.ts b/packages/compiler/test/render3/r3_view_compiler_template_spec.ts index c4b6fc504b..fbbb7555c5 100644 --- a/packages/compiler/test/render3/r3_view_compiler_template_spec.ts +++ b/packages/compiler/test/render3/r3_view_compiler_template_spec.ts @@ -51,28 +51,35 @@ describe('compiler compliance: template', () => { // The template should look like this (where IDENT is a wild card for an identifier): const template = ` - template:function MyComponent_Template(ctx:any, cm:boolean){ - if (cm) { + template:function MyComponent_Template(rf: IDENT, ctx: IDENT){ + if (rf & 1) { $i0$.ɵC(0, MyComponent_NgForOf_Template_0, null, _c0); } - $i0$.ɵp(0, 'ngForOf', $i0$.ɵb(ctx.items)); - function MyComponent_NgForOf_Template_0(ctx0:any, cm:boolean) { - if (cm) { + if (rf & 2) { + $i0$.ɵp(0, 'ngForOf', $i0$.ɵb(ctx.items)); + } + + function MyComponent_NgForOf_Template_0(rf: IDENT, ctx0: IDENT) { + if (rf & 1) { $i0$.ɵE(0, 'ul'); $i0$.ɵC(1, MyComponent_NgForOf_NgForOf_Template_1, null, _c0); $i0$.ɵe(); } - const $outer$ = ctx0.$implicit; - $i0$.ɵp(1, 'ngForOf', $i0$.ɵb($outer$.items)); - function MyComponent_NgForOf_NgForOf_Template_1(ctx1:any, cm:boolean) { - if (cm) { + if (rf & 2) { + const $outer$ = ctx0.$implicit; + $i0$.ɵp(1, 'ngForOf', $i0$.ɵb($outer$.items)); + } + function MyComponent_NgForOf_NgForOf_Template_1(rf: IDENT, ctx1: IDENT) { + if (rf & 1) { $i0$.ɵE(0, 'li'); $i0$.ɵC(1, MyComponent_NgForOf_NgForOf_NgForOf_Template_1, null, _c0); $i0$.ɵe(); } - $i0$.ɵp(1, 'ngForOf', $i0$.ɵb(ctx.items)); - function MyComponent_NgForOf_NgForOf_NgForOf_Template_1(ctx2:any, cm:boolean) { - if (cm) { + if (rf & 2) { + $i0$.ɵp(1, 'ngForOf', $i0$.ɵb(ctx.items)); + } + function MyComponent_NgForOf_NgForOf_NgForOf_Template_1(rf: IDENT, ctx2: IDENT) { + if (rf & 1) { $i0$.ɵE(0, 'div'); $i0$.ɵL('click', function MyComponent_NgForOf_NgForOf_NgForOf_Template_1_div_click_listener($event:any){ const $outer$ = ctx0.$implicit; @@ -83,11 +90,13 @@ describe('compiler compliance: template', () => { $i0$.ɵT(1); $i0$.ɵe(); } - const $outer$ = ctx0.$implicit; - const $middle$ = ctx1.$implicit; - const $inner$ = ctx2.$implicit; - $i0$.ɵp(0, 'title', ctx.format($outer$, $middle$, $inner$, ctx.component)); - $i0$.ɵt(1, $i0$.ɵi1(' ', ctx.format($outer$, $middle$, $inner$, ctx.component), ' ')); + if (rf & 2) { + const $outer$ = ctx0.$implicit; + const $middle$ = ctx1.$implicit; + const $inner$ = ctx2.$implicit; + $i0$.ɵp(0, 'title', ctx.format($outer$, $middle$, $inner$, ctx.component)); + $i0$.ɵt(1, $i0$.ɵi1(' ', ctx.format($outer$, $middle$, $inner$, ctx.component), ' ')); + } } } } @@ -99,4 +108,4 @@ describe('compiler compliance: template', () => { expectEmit(result.source, template, 'Incorrect template'); }); -}); \ No newline at end of file +}); diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 50e1fcb519..593577b095 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -149,11 +149,6 @@ let data: any[]; */ let directives: any[]|null; -/** - * Points to the next binding index to read or write to. - */ -let bindingIndex: number; - /** * When a view is destroyed, listeners need to be released and outputs need to be * unsubscribed. This cleanup array stores both listener data (in chunks of 4) @@ -204,7 +199,6 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null) const oldView: LView = currentView; data = newView && newView.data; directives = newView && newView.directives; - bindingIndex = newView && newView.bindingStartIndex || 0; tData = newView && newView.tView.data; creationMode = newView && (newView.flags & LViewFlags.CreationMode) === LViewFlags.CreationMode; firstTemplatePass = newView && newView.tView.firstTemplatePass; @@ -212,6 +206,10 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null) cleanup = newView && newView.cleanup; renderer = newView && newView.renderer; + if (newView && newView.bindingIndex < 0) { + newView.bindingIndex = newView.bindingStartIndex; + } + if (host != null) { previousOrParentNode = host; isParent = true; @@ -235,6 +233,7 @@ export function leaveView(newView: LView): void { // Views should be clean and in update mode after being checked, so these bits are cleared currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty); currentView.lifecycleStage = LifecycleStage.INIT; + currentView.bindingIndex = -1; enterView(newView, null); } @@ -295,7 +294,8 @@ export function createLView( child: null, tail: null, next: null, - bindingStartIndex: null, + bindingStartIndex: -1, + bindingIndex: -1, template: template, context: context, dynamicViewCount: 0, @@ -544,7 +544,8 @@ function getRenderFlags(view: LView): RenderFlags { export function elementStart( index: number, name: string, attrs?: string[] | null, localRefs?: string[] | null): RElement { ngDevMode && - assertNull(currentView.bindingStartIndex, 'elements should be created before any bindings'); + assertEqual( + currentView.bindingStartIndex, -1, 'elements should be created before any bindings'); const native: RElement = renderer.createElement(name); const node: LElementNode = createLNode(index, LNodeType.Element, native !, null); @@ -1160,7 +1161,8 @@ export function elementStyle( */ export function text(index: number, value?: any): void { ngDevMode && - assertNull(currentView.bindingStartIndex, 'text nodes should be created before bindings'); + assertEqual( + currentView.bindingStartIndex, -1, 'text nodes should be created before bindings'); const textNode = value != null ? createTextNode(value, renderer) : null; const node = createLNode(index, LNodeType.Element, textNode); // Text nodes are self closing. @@ -1259,7 +1261,8 @@ function addComponentLogic(index: number, instance: T, def: ComponentDef): export function baseDirectiveCreate( index: number, directive: T, directiveDef: DirectiveDef| ComponentDef): T { ngDevMode && - assertNull(currentView.bindingStartIndex, 'directives should be created before any bindings'); + assertEqual( + currentView.bindingStartIndex, -1, 'directives should be created before any bindings'); ngDevMode && assertPreviousIsParent(); Object.defineProperty( @@ -1381,9 +1384,9 @@ export function createLContainer( export function container( index: number, template?: ComponentTemplate, tagName?: string, attrs?: string[], localRefs?: string[] | null): void { - ngDevMode && - assertNull( - currentView.bindingStartIndex, 'container nodes should be created before any bindings'); + ngDevMode && assertEqual( + currentView.bindingStartIndex, -1, + 'container nodes should be created before any bindings'); const currentParent = isParent ? previousOrParentNode : previousOrParentNode.parent !; const lContainer = createLContainer(currentParent, currentView, template); @@ -1976,7 +1979,13 @@ export const NO_CHANGE = {} as NO_CHANGE; * (ie `bind()`, `interpolationX()`, `pureFunctionX()`) */ function initBindings() { - bindingIndex = currentView.bindingStartIndex = data.length; + ngDevMode && assertEqual( + currentView.bindingStartIndex, -1, + 'Binding start index should only be set once, when null'); + ngDevMode && assertEqual( + currentView.bindingIndex, -1, + 'Binding index should not yet be set ' + currentView.bindingIndex); + currentView.bindingIndex = currentView.bindingStartIndex = data.length; } /** @@ -1985,17 +1994,19 @@ function initBindings() { * @param value Value to diff */ export function bind(value: T | NO_CHANGE): T|NO_CHANGE { - if (currentView.bindingStartIndex == null) { + if (currentView.bindingStartIndex < 0) { initBindings(); - return data[bindingIndex++] = value; + return data[currentView.bindingIndex++] = value; } - const changed: boolean = value !== NO_CHANGE && isDifferent(data[bindingIndex], value); + const changed: boolean = + value !== NO_CHANGE && isDifferent(data[currentView.bindingIndex], value); if (changed) { - throwErrorIfNoChangesMode(creationMode, checkNoChangesMode, data[bindingIndex], value); - data[bindingIndex] = value; + throwErrorIfNoChangesMode( + creationMode, checkNoChangesMode, data[currentView.bindingIndex], value); + data[currentView.bindingIndex] = value; } - bindingIndex++; + currentView.bindingIndex++; return changed ? value : NO_CHANGE; } @@ -2159,26 +2170,28 @@ export function loadDirective(index: number): T { /** Gets the current binding value and increments the binding index. */ export function consumeBinding(): any { - ngDevMode && assertDataInRange(bindingIndex); + ngDevMode && assertDataInRange(currentView.bindingIndex); ngDevMode && - assertNotEqual(data[bindingIndex], NO_CHANGE, 'Stored value should never be NO_CHANGE.'); - return data[bindingIndex++]; + assertNotEqual( + data[currentView.bindingIndex], NO_CHANGE, 'Stored value should never be NO_CHANGE.'); + return data[currentView.bindingIndex++]; } /** Updates binding if changed, then returns whether it was updated. */ export function bindingUpdated(value: any): boolean { ngDevMode && assertNotEqual(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.'); - if (currentView.bindingStartIndex == null) { + if (currentView.bindingStartIndex < 0) { initBindings(); - } else if (isDifferent(data[bindingIndex], value)) { - throwErrorIfNoChangesMode(creationMode, checkNoChangesMode, data[bindingIndex], value); + } else if (isDifferent(data[currentView.bindingIndex], value)) { + throwErrorIfNoChangesMode( + creationMode, checkNoChangesMode, data[currentView.bindingIndex], value); } else { - bindingIndex++; + currentView.bindingIndex++; return false; } - data[bindingIndex++] = value; + data[currentView.bindingIndex++] = value; return true; } diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 603234445b..68bfce83c9 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -61,7 +61,16 @@ export interface LView { * will begin reading bindings at the correct point in the array when * we are in update mode. */ - bindingStartIndex: number|null; + bindingStartIndex: number; + + /** + * The binding index we should access next. + * + * This is stored so that bindings can continue where they left off + * if a view is left midway through processing bindings (e.g. if there is + * a setter that creates an embedded view, like in ngIf). + */ + bindingIndex: number; /** * When a view is destroyed, listeners need to be released and outputs need to be diff --git a/packages/core/test/bundling/todo/index.ts b/packages/core/test/bundling/todo/index.ts index 6e0d05b88a..99c39cf831 100644 --- a/packages/core/test/bundling/todo/index.ts +++ b/packages/core/test/bundling/todo/index.ts @@ -56,8 +56,6 @@ export class TodoStore { @Component({ selector: 'todo-app', - // TODO(misko) remove all `foo && foo.something` once `ViewContainerRef` can separate creation and - // update block. // TODO(misko): make this work with `[(ngModel)]` template: `
@@ -74,18 +72,18 @@ export class TodoStore { (click)="todoStore.setAllTo(toggleall.checked)">
  • + [class.completed]="todo.completed" + [class.editing]="todo.editing">
    - + [checked]="todo.completed"> +
    -
`, - // TODO(misko): switch oven to OnPush + // TODO(misko): switch over to OnPush // changeDetection: ChangeDetectionStrategy.OnPush }) export class ToDoAppComponent { diff --git a/packages/core/test/render3/common_integration_spec.ts b/packages/core/test/render3/common_integration_spec.ts index 511dd1f938..d09acb934a 100644 --- a/packages/core/test/render3/common_integration_spec.ts +++ b/packages/core/test/render3/common_integration_spec.ts @@ -11,7 +11,7 @@ import {NgForOfContext} from '@angular/common'; import {defineComponent} from '../../src/render3/index'; import {bind, container, elementEnd, elementProperty, elementStart, interpolation3, text, textBinding, tick} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; -import {NgForOf} from './common_with_def'; +import {NgForOf, NgIf} from './common_with_def'; import {ComponentFixture} from './render_util'; describe('@angular/common integration', () => { @@ -124,4 +124,65 @@ describe('@angular/common integration', () => { }); }); + + describe('ngIf', () => { + it('should support sibling ngIfs', () => { + class MyApp { + showing = true; + valueOne = 'one'; + valueTwo = 'two'; + + static ngComponentDef = defineComponent({ + type: MyApp, + factory: () => new MyApp(), + selectors: [['my-app']], + /** + *
{{ valueOne }}
+ *
{{ valueTwo }}
+ */ + template: (rf: RenderFlags, myApp: MyApp) => { + if (rf & RenderFlags.Create) { + container(0, templateOne, undefined, ['ngIf', '']); + container(1, templateTwo, undefined, ['ngIf', '']); + } + if (rf & RenderFlags.Update) { + elementProperty(0, 'ngIf', bind(myApp.showing)); + elementProperty(1, 'ngIf', bind(myApp.showing)); + } + + function templateOne(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div'); + { text(1); } + elementEnd(); + } + if (rf & RenderFlags.Update) { + textBinding(1, bind(myApp.valueOne)); + } + } + function templateTwo(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div'); + { text(1); } + elementEnd(); + } + if (rf & RenderFlags.Update) { + textBinding(1, bind(myApp.valueTwo)); + } + } + }, + directives: () => [NgIf] + }); + } + + const fixture = new ComponentFixture(MyApp); + expect(fixture.html).toEqual('
one
two
'); + + fixture.component.valueOne = '$$one$$'; + fixture.component.valueTwo = '$$two$$'; + fixture.update(); + expect(fixture.html).toEqual('
$$one$$
$$two$$
'); + }); + + }); }); diff --git a/packages/core/test/render3/common_with_def.ts b/packages/core/test/render3/common_with_def.ts index 9a60669cdb..c046b617a6 100644 --- a/packages/core/test/render3/common_with_def.ts +++ b/packages/core/test/render3/common_with_def.ts @@ -6,13 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {NgForOf as NgForOfDef} from '@angular/common'; +import {NgForOf as NgForOfDef, NgIf as NgIfDef} from '@angular/common'; import {IterableDiffers} from '@angular/core'; import {defaultIterableDiffers} from '../../src/change_detection/change_detection'; import {DirectiveType, InjectFlags, NgOnChangesFeature, defineDirective, directiveInject, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; export const NgForOf: DirectiveType> = NgForOfDef as any; +export const NgIf: DirectiveType = NgIfDef as any; NgForOf.ngDirectiveDef = defineDirective({ type: NgForOfDef, @@ -27,3 +28,10 @@ NgForOf.ngDirectiveDef = defineDirective({ ngForTemplate: 'ngForTemplate', } }); + +(NgIf as any).ngDirectiveDef = defineDirective({ + type: NgIfDef, + selectors: [['', 'ngIf', '']], + factory: () => new NgIfDef(injectViewContainerRef(), injectTemplateRef()), + inputs: {ngIf: 'ngIf', ngIfThen: 'ngIfThen', ngIfElse: 'ngIfElse'} +});