From 3b9bc73db43d60b908718bc0277db711328d0c58 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Sat, 27 Oct 2018 15:25:25 -0700 Subject: [PATCH] fix(ivy): host bindings after dirs without host bindings should work (#26801) PR Close #26801 --- packages/core/src/render3/instructions.ts | 7 +- .../bundle.golden_symbols.json | 3 + .../hello_world/bundle.golden_symbols.json | 3 + .../hello_world_r2/bundle.golden_symbols.json | 3 + .../bundling/todo/bundle.golden_symbols.json | 3 + .../todo_r2/bundle.golden_symbols.json | 3 + .../core/test/render3/host_binding_spec.ts | 188 ++++++++---------- 7 files changed, 101 insertions(+), 109 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 2a88e01e9e..9e70d21bff 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -14,6 +14,7 @@ import {QueryList} from '../linker'; import {Sanitizer} from '../sanitization/security'; import {StyleSanitizeFn} from '../sanitization/style_sanitizer'; import {Type} from '../type'; +import {noop} from '../util/noop'; import {assertDefined, assertEqual, assertLessThan, assertNotEqual} from './assert'; import {attachPatchData, getComponentViewByInstance} from './context_discovery'; @@ -41,7 +42,6 @@ import {NO_CHANGE} from './tokens'; import {getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isDifferent, loadInternal, readPatchedLViewData, stringify} from './util'; - /** * A permanent marker promise which signifies that the current CD tree is * clean. @@ -1490,7 +1490,8 @@ export function queueComponentIndexForCheck(previousOrParentTNode: TNode): void function queueHostBindingForCheck(tView: TView, def: DirectiveDef| ComponentDef): void { ngDevMode && assertEqual(getFirstTemplatePass(), true, 'Should only be called in first template pass.'); - tView.expandoInstructions !.push(def.hostBindings !, def.hostVars); + tView.expandoInstructions !.push(def.hostBindings || noop); + if (def.hostVars) tView.expandoInstructions !.push(def.hostVars); } /** Caches local names and their matching directive indices for query and template lookups. */ @@ -1552,7 +1553,7 @@ function baseResolveDirective( tView.blueprint.push(nodeInjectorFactory); viewData.push(nodeInjectorFactory); - if (def.hostBindings) queueHostBindingForCheck(tView, def); + queueHostBindingForCheck(tView, def); } function addComponentLogic( diff --git a/packages/core/test/bundling/animation_world/bundle.golden_symbols.json b/packages/core/test/bundling/animation_world/bundle.golden_symbols.json index 8f9a6ef688..d182067987 100644 --- a/packages/core/test/bundling/animation_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animation_world/bundle.golden_symbols.json @@ -938,6 +938,9 @@ { "name": "noSideEffects" }, + { + "name": "noop" + }, { "name": "pointers" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 627588631d..68b7707cb4 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -362,6 +362,9 @@ { "name": "noSideEffects" }, + { + "name": "noop" + }, { "name": "postProcessBaseDirective" }, diff --git a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json index 309cd1a702..5a173f3ad6 100644 --- a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json @@ -4016,6 +4016,9 @@ { "name": "nodeValue" }, + { + "name": "noop" + }, { "name": "noop$1" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 4750ece4a0..e0fc08ffe4 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -956,6 +956,9 @@ { "name": "noSideEffects" }, + { + "name": "noop" + }, { "name": "pointers" }, diff --git a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json index fe8044f8aa..1aad004320 100644 --- a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json @@ -2240,6 +2240,9 @@ { "name": "noSideEffects" }, + { + "name": "noop" + }, { "name": "noop$1" }, diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts index 6f5eea76aa..ae741c8161 100644 --- a/packages/core/test/render3/host_binding_spec.ts +++ b/packages/core/test/render3/host_binding_spec.ts @@ -16,8 +16,14 @@ import {pureFunction1, pureFunction2} from '../../src/render3/pure_function'; import {ComponentFixture, TemplateFixture, createComponent, createDirective} from './render_util'; import {NgForOf} from './common_with_def'; -describe('host', () => { - let nameComp !: NameComp; +describe('host bindings', () => { + let nameComp: NameComp|null; + let hostBindingDir: HostBindingDir|null; + + beforeEach(() => { + nameComp = null; + hostBindingDir = null; + }); class NameComp { names !: string[]; @@ -33,6 +39,40 @@ describe('host', () => { }); } + class HostBindingDir { + // @HostBinding() + id = 'foo'; + + static ngDirectiveDef = defineDirective({ + type: HostBindingDir, + selectors: [['', 'hostBindingDir', '']], + factory: () => hostBindingDir = new HostBindingDir(), + hostVars: 1, + hostBindings: (directiveIndex: number, elementIndex: number) => { + elementProperty(elementIndex, 'id', bind(load(directiveIndex).id)); + } + }); + } + + class HostBindingComp { + // @HostBinding() + id = 'my-id'; + + static ngComponentDef = defineComponent({ + type: HostBindingComp, + selectors: [['host-binding-comp']], + factory: () => new HostBindingComp(), + consts: 0, + vars: 0, + hostVars: 1, + hostBindings: (dirIndex: number, elIndex: number) => { + const ctx = load(dirIndex) as HostBindingComp; + elementProperty(elIndex, 'id', bind(ctx.id)); + }, + template: (rf: RenderFlags, ctx: HostBindingComp) => {} + }); + } + it('should support host bindings in directives', () => { let directiveInstance: Directive|undefined; @@ -62,25 +102,6 @@ describe('host', () => { }); it('should support host bindings on root component', () => { - class HostBindingComp { - // @HostBinding() - id = 'my-id'; - - static ngComponentDef = defineComponent({ - type: HostBindingComp, - selectors: [['host-binding-comp']], - factory: () => new HostBindingComp(), - consts: 0, - vars: 0, - hostVars: 1, - hostBindings: (dirIndex: number, elIndex: number) => { - const instance = load(dirIndex) as HostBindingComp; - elementProperty(elIndex, 'id', bind(instance.id)); - }, - template: (rf: RenderFlags, ctx: HostBindingComp) => {} - }); - } - const fixture = new ComponentFixture(HostBindingComp); expect(fixture.hostElement.id).toBe('my-id'); @@ -132,84 +153,73 @@ describe('host', () => { }); it('should support host bindings on multiple nodes', () => { - let hostBindingDir !: HostBindingDir; - - class HostBindingDir { - // @HostBinding() - id = 'foo'; - - static ngDirectiveDef = defineDirective({ - type: HostBindingDir, - selectors: [['', 'hostBindingDir', '']], - factory: () => hostBindingDir = new HostBindingDir(), - hostVars: 1, - hostBindings: (directiveIndex: number, elementIndex: number) => { - elementProperty(elementIndex, 'id', bind(load(directiveIndex).id)); - } - }); - } - const SomeDir = createDirective('someDir'); - class HostBindingComp { + class HostTitleComp { // @HostBinding() title = 'my-title'; static ngComponentDef = defineComponent({ - type: HostBindingComp, - selectors: [['host-binding-comp']], - factory: () => new HostBindingComp(), + type: HostTitleComp, + selectors: [['host-title-comp']], + factory: () => new HostTitleComp(), consts: 0, vars: 0, hostVars: 1, hostBindings: (dirIndex: number, elIndex: number) => { - const ctx = load(dirIndex) as HostBindingComp; + const ctx = load(dirIndex) as HostTitleComp; elementProperty(elIndex, 'title', bind(ctx.title)); }, - template: (rf: RenderFlags, ctx: HostBindingComp) => {} + template: (rf: RenderFlags, ctx: HostTitleComp) => {} }); } /** *
*
- * + * */ const App = createComponent('app', (rf: RenderFlags, ctx: any) => { if (rf & RenderFlags.Create) { element(0, 'div', ['hostBindingDir', '']); element(1, 'div', ['someDir', '']); - element(2, 'host-binding-comp'); + element(2, 'host-title-comp'); } - }, 3, 0, [HostBindingDir, SomeDir, HostBindingComp]); + }, 3, 0, [HostBindingDir, SomeDir, HostTitleComp]); const fixture = new ComponentFixture(App); const hostBindingDiv = fixture.hostElement.querySelector('div') as HTMLElement; - const hostBindingComp = fixture.hostElement.querySelector('host-binding-comp') as HTMLElement; + const hostTitleComp = fixture.hostElement.querySelector('host-title-comp') as HTMLElement; expect(hostBindingDiv.id).toEqual('foo'); - expect(hostBindingComp.title).toEqual('my-title'); + expect(hostTitleComp.title).toEqual('my-title'); - hostBindingDir.id = 'bar'; + hostBindingDir !.id = 'bar'; fixture.update(); expect(hostBindingDiv.id).toEqual('bar'); }); + it('should support dirs with host bindings on the same node as dirs without host bindings', + () => { + const SomeDir = createDirective('someDir'); + + /**
*/ + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'div', ['someDir', '', 'hostBindingDir', '']); + } + }, 1, 0, [SomeDir, HostBindingDir]); + + const fixture = new ComponentFixture(App); + const hostBindingDiv = fixture.hostElement.querySelector('div') as HTMLElement; + expect(hostBindingDiv.id).toEqual('foo'); + + hostBindingDir !.id = 'bar'; + fixture.update(); + expect(hostBindingDiv.id).toEqual('bar'); + }); + + it('should support host bindings on second template pass', () => { - class HostBindingDir { - // @HostBinding() - id = 'foo'; - - static ngDirectiveDef = defineDirective({ - type: HostBindingDir, - selectors: [['', 'hostBindingDir', '']], - factory: () => new HostBindingDir(), - hostVars: 1, - hostBindings: (directiveIndex: number, elementIndex: number) => { - elementProperty(elementIndex, 'id', bind(load(directiveIndex).id)); - } - }); - } - /**
*/ const Parent = createComponent('parent', (rf: RenderFlags, ctx: any) => { if (rf & RenderFlags.Create) { @@ -235,21 +245,6 @@ describe('host', () => { }); it('should support host bindings in for loop', () => { - class HostBindingDir { - // @HostBinding() - id = 'foo'; - - static ngDirectiveDef = defineDirective({ - type: HostBindingDir, - selectors: [['', 'hostBindingDir', '']], - factory: () => new HostBindingDir(), - hostVars: 1, - hostBindings: (directiveIndex: number, elementIndex: number) => { - elementProperty(elementIndex, 'id', bind(load(directiveIndex).id)); - } - }); - } - function NgForTemplate(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { elementStart(0, 'div'); @@ -285,25 +280,6 @@ describe('host', () => { it('should support component with host bindings and array literals', () => { const ff = (v: any) => ['Nancy', v, 'Ned']; - class HostBindingComp { - // @HostBinding() - id = 'my-id'; - - static ngComponentDef = defineComponent({ - type: HostBindingComp, - selectors: [['host-binding-comp']], - factory: () => new HostBindingComp(), - consts: 0, - vars: 0, - hostVars: 1, - hostBindings: (dirIndex: number, elIndex: number) => { - const ctx = load(dirIndex) as HostBindingComp; - elementProperty(elIndex, 'id', bind(ctx.id)); - }, - template: (rf: RenderFlags, ctx: HostBindingComp) => {} - }); - } - /** * * @@ -323,16 +299,16 @@ describe('host', () => { fixture.component.name = 'Betty'; fixture.update(); expect(hostBindingEl.id).toBe('my-id'); - expect(nameComp.names).toEqual(['Nancy', 'Betty', 'Ned']); + expect(nameComp !.names).toEqual(['Nancy', 'Betty', 'Ned']); - const firstArray = nameComp.names; + const firstArray = nameComp !.names; fixture.update(); - expect(firstArray).toBe(nameComp.names); + expect(firstArray).toBe(nameComp !.names); fixture.component.name = 'my-id'; fixture.update(); expect(hostBindingEl.id).toBe('my-id'); - expect(nameComp.names).toEqual(['Nancy', 'my-id', 'Ned']); + expect(nameComp !.names).toEqual(['Nancy', 'my-id', 'Ned']); }); // Note: This is a contrived example. For feature parity with render2, we should make sure it @@ -403,11 +379,11 @@ describe('host', () => { expect(hostBindingEl.id).toBe('red,blue'); expect(hostBindingEl.dir).toBe('ltr'); expect(hostBindingEl.title).toBe('my title,other title'); - expect(nameComp.names).toEqual(['Frank', 'Nancy', 'Joe']); + expect(nameComp !.names).toEqual(['Frank', 'Nancy', 'Joe']); - const firstArray = nameComp.names; + const firstArray = nameComp !.names; fixture.update(); - expect(firstArray).toBe(nameComp.names); + expect(firstArray).toBe(nameComp !.names); hostBindingComp.id = 'green'; hostBindingComp.dir = 'rtl';