fix(ivy): allow insertion of views attached to a different container (#34156)

Fixes #34152

PR Close #34156
This commit is contained in:
Pawel Kozlowski 2019-11-29 21:59:57 +01:00 committed by Miško Hevery
parent 2cf25facd9
commit 5a52990b91
2 changed files with 74 additions and 18 deletions

View File

@ -16,16 +16,16 @@ import {ViewContainerRef as ViewEngine_ViewContainerRef} from '../linker/view_co
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_ViewRef} from '../linker/view_ref';
import {Renderer2} from '../render/api';
import {addToArray, removeFromArray} from '../util/array_utils';
import {assertDefined, assertGreaterThan, assertLessThan} from '../util/assert';
import {assertDefined, assertEqual, assertGreaterThan, assertLessThan} from '../util/assert';
import {assertLContainer} from './assert';
import {NodeInjector, getParentInjectorLocation} from './di';
import {addToViewTree, createLContainer, createLView, renderView} from './instructions/shared';
import {ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './interfaces/container';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
import {TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer';
import {isComponentHost, isLContainer, isLView, isRootView} from './interfaces/type_checks';
import {DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, LView, LViewFlags, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
import {DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, LView, LViewFlags, PARENT, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
import {assertNodeOfPossibleTypes} from './node_assert';
import {addRemoveViewFromContainer, appendChild, detachView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
import {getParentInjectorTNode} from './node_util';
@ -240,18 +240,40 @@ export function createContainerRef(
}
insert(viewRef: viewEngine_ViewRef, index?: number): viewEngine_ViewRef {
const lView = (viewRef as ViewRef<any>)._lView !;
if (viewRef.destroyed) {
throw new Error('Cannot insert a destroyed View in a ViewContainer!');
}
this.allocateContainerIfNeeded();
const lView = (viewRef as ViewRef<any>)._lView !;
if (viewAttachedToContainer(lView)) {
// If view is already attached, fall back to move() so we clean up
// references appropriately.
// Note that we "shift" -1 because the move will involve inserting
// one view but also removing one view.
return this.move(viewRef, this._adjustIndex(index, -1));
// If view is already attached, detach it first so we clean up references appropriately.
const prevIdx = this.indexOf(viewRef);
// A view might be attached either to this or a different container. The `prevIdx` for
// those cases will be:
// equal to -1 for views attached to this ViewContainerRef
// >= 0 for views attached to a different ViewContainerRef
if (prevIdx !== -1) {
this.detach(prevIdx);
} else {
const prevLContainer = lView[PARENT] as LContainer;
ngDevMode && assertEqual(
isLContainer(prevLContainer), true,
'An attached view should have its PARENT point to a container.');
// We need to re-create a R3ViewContainerRef instance since those are not stored on
// LView (nor anywhere else).
const prevVCRef = new R3ViewContainerRef(
prevLContainer, prevLContainer[T_HOST] as TDirectiveHostNode,
prevLContainer[PARENT]);
prevVCRef.detach(prevVCRef.indexOf(viewRef));
}
}
const adjustedIdx = this._adjustIndex(index);
@ -270,14 +292,7 @@ export function createContainerRef(
if (viewRef.destroyed) {
throw new Error('Cannot move a destroyed View in a ViewContainer!');
}
const index = this.indexOf(viewRef);
if (index === -1) {
this.insert(viewRef, newIndex);
} else if (index !== newIndex) {
this.detach(index);
this.insert(viewRef, newIndex);
}
return viewRef;
return this.insert(viewRef, newIndex);
}
indexOf(viewRef: viewEngine_ViewRef): number {
@ -307,7 +322,7 @@ export function createContainerRef(
return this.length + shift;
}
if (ngDevMode) {
assertGreaterThan(index, -1, 'index must be positive');
assertGreaterThan(index, -1, `ViewRef index must be positive, got ${index}`);
// +1 because it's legal to insert at the end.
assertLessThan(index, this.length + 1 + shift, 'index');
}

View File

@ -220,6 +220,47 @@ describe('ViewContainerRef', () => {
viewContainerRef.insert(ref0);
expect(fixture.nativeElement.textContent).toEqual('0');
});
it('should insert a view already inserted into another container', () => {
@Component({
selector: 'test-cmpt',
template: `
<ng-template #t>content</ng-template>
before|<ng-template #c1></ng-template>|middle|<ng-template #c2></ng-template>|after
`
})
class TestComponent {
@ViewChild('t', {static: true}) t !: TemplateRef<{}>;
@ViewChild('c1', {static: true, read: ViewContainerRef}) c1 !: ViewContainerRef;
@ViewChild('c2', {static: true, read: ViewContainerRef}) c2 !: ViewContainerRef;
}
TestBed.configureTestingModule({declarations: [TestComponent]});
const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();
const cmpt = fixture.componentInstance;
const native = fixture.nativeElement;
expect(native.textContent.trim()).toEqual('before||middle||after');
// create and insert an embedded view into the c1 container
const viewRef = cmpt.c1.createEmbeddedView(cmpt.t, {});
expect(native.textContent.trim()).toEqual('before|content|middle||after');
expect(cmpt.c1.indexOf(viewRef)).toBe(0);
expect(cmpt.c2.indexOf(viewRef)).toBe(-1);
// move the existing embedded view into the c2 container
cmpt.c2.insert(viewRef);
expect(native.textContent.trim()).toEqual('before||middle|content|after');
// VE has a bug where a view moved between containers is not correctly detached from the
// previous container. Check https://github.com/angular/angular/issues/20824 for more details.
if (ivyEnabled) {
expect(cmpt.c1.indexOf(viewRef)).toBe(-1);
}
expect(cmpt.c2.indexOf(viewRef)).toBe(0);
});
});
describe('move', () => {