From 51dfdd5dd1745900147cf335403694f968d9d9a8 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 4 Oct 2018 13:28:39 -0700 Subject: [PATCH] fix(ivy): sync view with blueprint when necessary (#26263) PR Close #26263 --- packages/core/src/render3/di.ts | 22 ++++++--- packages/core/src/render3/instructions.ts | 47 +++++++++++++++++-- .../bundle.golden_symbols.json | 6 +++ .../hello_world/bundle.golden_symbols.json | 6 +++ .../bundling/todo/bundle.golden_symbols.json | 6 +++ .../todo_r2/bundle.golden_symbols.json | 6 +++ packages/core/test/render3/properties_spec.ts | 44 ++++++++++++++++- 7 files changed, 125 insertions(+), 12 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 5f0b258aa6..568b0b6ad0 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -106,8 +106,9 @@ export function getOrCreateNodeInjectorForNode( if (tView.firstTemplatePass) { // TODO(kara): Store node injector with host bindings for that node (see VIEW_DATA.md) tNode.injectorIndex = hostView.length; - tView.blueprint.push(0, 0, 0, 0, 0, 0, 0, 0, null); // foundation for cumulative bloom - tView.data.push(0, 0, 0, 0, 0, 0, 0, 0, tNode); // foundation for node bloom + setUpBloom(tView.data, tNode); // foundation for node bloom + setUpBloom(hostView, null); // foundation for cumulative bloom + setUpBloom(tView.blueprint, null); tView.hostBindingStartIndex += INJECTOR_SIZE; } @@ -118,16 +119,25 @@ export function getOrCreateNodeInjectorForNode( const parentData = parentView[TVIEW].data as any; const injectorIndex = tNode.injectorIndex; - for (let i = 0; i < PARENT_INJECTOR; i++) { - const bloomIndex = parentIndex + i; - hostView[injectorIndex + i] = - parentLoc === -1 ? 0 : parentView[bloomIndex] | parentData[bloomIndex]; + // If a parent injector can't be found, its location is set to -1. + // In that case, we don't need to set up a cumulative bloom + if (parentLoc !== -1) { + for (let i = 0; i < PARENT_INJECTOR; i++) { + const bloomIndex = parentIndex + i; + // Creates a cumulative bloom filter that merges the parent's bloom filter + // and its own cumulative bloom (which contains tokens for all ancestors) + hostView[injectorIndex + i] = parentView[bloomIndex] | parentData[bloomIndex]; + } } hostView[injectorIndex + PARENT_INJECTOR] = parentLoc; return injectorIndex; } +function setUpBloom(arr: any[], footer: TNode | null) { + arr.push(0, 0, 0, 0, 0, 0, 0, 0, footer); +} + export function getInjectorIndex(tNode: TNode, hostView: LViewData): number { if (tNode.injectorIndex === -1 || // If the injector index is the same as its parent's injector index, then the index has been diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index e98db205ee..5a5034fbbe 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -335,6 +335,7 @@ export function leaveView(newView: LViewData, creationOnly?: boolean): void { */ function refreshDescendantViews() { setHostBindings(tView.hostBindings); + const parentFirstTemplatePass = firstTemplatePass; // This needs to be set before children are processed to support recursive components tView.firstTemplatePass = firstTemplatePass = false; @@ -351,7 +352,7 @@ function refreshDescendantViews() { executeHooks(directives !, tView.contentHooks, tView.contentCheckHooks, creationMode); } - refreshChildComponents(tView.components); + refreshChildComponents(tView.components, parentFirstTemplatePass); } @@ -388,10 +389,11 @@ function refreshContentQueries(tView: TView): void { } /** Refreshes child components in the current view. */ -function refreshChildComponents(components: number[] | null): void { +function refreshChildComponents( + components: number[] | null, parentFirstTemplatePass: boolean): void { if (components != null) { for (let i = 0; i < components.length; i++) { - componentRefresh(components[i]); + componentRefresh(components[i], parentFirstTemplatePass); } } } @@ -701,7 +703,7 @@ export function renderComponentOrTemplate( // Element was stored at 0 in data and directive was stored at 0 in directives // in renderComponent() setHostBindings(tView.hostBindings); - componentRefresh(HEADER_OFFSET); + componentRefresh(HEADER_OFFSET, false); } } finally { if (rendererFactory.end) { @@ -2220,7 +2222,8 @@ export function embeddedViewEnd(): void { * * @param adjustedElementIndex Element index in LViewData[] (adjusted for HEADER_OFFSET) */ -export function componentRefresh(adjustedElementIndex: number): void { +export function componentRefresh( + adjustedElementIndex: number, parentFirstTemplatePass: boolean): void { ngDevMode && assertDataInRange(adjustedElementIndex); const element = readElementValue(viewData[adjustedElementIndex]) as LElementNode; ngDevMode && assertNodeType(tView.data[adjustedElementIndex] as TNode, TNodeType.Element); @@ -2230,10 +2233,44 @@ export function componentRefresh(adjustedElementIndex: number): void { // Only attached CheckAlways components or attached, dirty OnPush components should be checked if (viewAttached(hostView) && hostView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { + parentFirstTemplatePass && syncViewWithBlueprint(hostView); detectChangesInternal(hostView, hostView[CONTEXT]); } } +/** + * Syncs an LViewData instance with its blueprint if they have gotten out of sync. + * + * Typically, blueprints and their view instances should always be in sync, so the loop here + * will be skipped. However, consider this case of two components side-by-side: + * + * App template: + * ``` + * + * + * ``` + * + * The following will happen: + * 1. App template begins processing. + * 2. First is matched as a component and its LViewData is created. + * 3. Second is matched as a component and its LViewData is created. + * 4. App template completes processing, so it's time to check child templates. + * 5. First template is checked. It has a directive, so its def is pushed to blueprint. + * 6. Second template is checked. Its blueprint has been updated by the first + * template, but its LViewData was created before this update, so it is out of sync. + * + * Note that embedded views inside ngFor loops will never be out of sync because these views + * are processed as soon as they are created. + * + * @param componentView The view to sync + */ +function syncViewWithBlueprint(componentView: LViewData) { + const componentTView = componentView[TVIEW]; + for (let i = componentView.length; i < componentTView.blueprint.length; i++) { + componentView[i] = componentTView.blueprint[i]; + } +} + /** Returns a boolean for whether the view is attached */ export function viewAttached(view: LViewData): boolean { return (view[FLAGS] & LViewFlags.Attached) === LViewFlags.Attached; 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 0bb9a92705..897b025621 100644 --- a/packages/core/test/bundling/animation_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animation_world/bundle.golden_symbols.json @@ -965,6 +965,9 @@ { "name": "setUpAttributes" }, + { + "name": "setUpBloom" + }, { "name": "setValue" }, @@ -983,6 +986,9 @@ { "name": "swapMultiContextEntries" }, + { + "name": "syncViewWithBlueprint" + }, { "name": "template" }, 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 8893f93e3f..a664681e74 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -344,9 +344,15 @@ { "name": "setUpAttributes" }, + { + "name": "setUpBloom" + }, { "name": "stringify$2" }, + { + "name": "syncViewWithBlueprint" + }, { "name": "text" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 3a1fa34240..e7b5340098 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -986,6 +986,9 @@ { "name": "setUpAttributes" }, + { + "name": "setUpBloom" + }, { "name": "setValue" }, @@ -1001,6 +1004,9 @@ { "name": "stringify$2" }, + { + "name": "syncViewWithBlueprint" + }, { "name": "template" }, 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 8a808af482..61bfc73faa 100644 --- a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json @@ -2339,6 +2339,9 @@ { "name": "setUpAttributes" }, + { + "name": "setUpBloom" + }, { "name": "setValue" }, @@ -2402,6 +2405,9 @@ { "name": "symbolNames" }, + { + "name": "syncViewWithBlueprint" + }, { "name": "tagSet" }, diff --git a/packages/core/test/render3/properties_spec.ts b/packages/core/test/render3/properties_spec.ts index 3f004c1ffd..5ffbff23c7 100644 --- a/packages/core/test/render3/properties_spec.ts +++ b/packages/core/test/render3/properties_spec.ts @@ -8,7 +8,7 @@ import {EventEmitter} from '@angular/core'; -import {AttributeMarker, defineComponent, defineDirective, tick} from '../../src/render3/index'; +import {AttributeMarker, PublicFeature, defineComponent, defineDirective} from '../../src/render3/index'; import {NO_CHANGE, bind, container, containerRefreshEnd, containerRefreshStart, element, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, listener, loadDirective, reference, text, textBinding} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; import {pureFunction1, pureFunction2} from '../../src/render3/pure_function'; @@ -153,6 +153,48 @@ describe('elementProperty', () => { expect(fixture.hostElement.id).toBe('other-id'); }); + 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(loadDirective(directiveIndex).id)); + }, + features: [PublicFeature] + }); + } + + /**
*/ + const Parent = createComponent('parent', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'div', ['hostBindingDir', '']); + } + }, 1, 0, [HostBindingDir]); + + /** + * + * + */ + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'parent'); + element(1, 'parent'); + } + }, 2, 0, [Parent]); + + const fixture = new ComponentFixture(App); + const divs = fixture.hostElement.querySelectorAll('div'); + expect(divs[0].id).toEqual('foo'); + expect(divs[1].id).toEqual('foo'); + }); + it('should support component with host bindings and array literals', () => { const ff = (v: any) => ['Nancy', v, 'Ned'];