honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): return new ViewRef when detaching view (#27437) 

When detaching a view by its index via `ViewContainerRef.detach(index)`, in `ViewEngine` we used to return a new `ViewRef` ([for reference](https://github.com/angular/angular/blob/master/packages/core/src/view/refs.ts#L227)), however in Ivy we return the same `ViewRef` which means that its internal `_viewContainerRef` is never reset and we'll throw an error if the consumer tried to attach it to the `ApplicationRef`. These changes return a new `ViewRef` in order to match the original behavior.

These changes also add the same errors as `ViewEngine` when attempting to attach a view that is attached already. This was the original goal of this PR, however it ended up uncovering the issues with the `ViewRef`.

PR Close #27437
This commit is contained in:
Kristiyan Kostadinov 2018-12-04 11:56:24 +01:00 committed by Igor Minar
parent 7524c99be2
commit 862697d4bd
4 changed files with 36 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
packages/core/src/render3/node_manipulation.ts
View File

@ -369,8 +369,9 @@ export function insertView(
* @param lContainer The container from which to detach a view
* @param removeIndex The index of the view to detach
* @param detached Whether or not this view is already detached.
* @returns Detached LView instance.
*/
export function detachView(lContainer: LContainer, removeIndex: number, detached: boolean) {
export function detachView(lContainer: LContainer, removeIndex: number, detached: boolean): LView {
const views = lContainer[VIEWS];
const viewToDetach = views[removeIndex];
if (removeIndex > 0) {
@ -388,6 +389,7 @@ export function detachView(lContainer: LContainer, removeIndex: number, detached
viewToDetach[PARENT] = null;
// Unsets the attached flag
viewToDetach[FLAGS] &= ~LViewFlags.Attached;
return viewToDetach;
}
/**

7
packages/core/src/render3/view_engine_compatibility.ts
View File

@ -25,7 +25,7 @@ import {RenderFlags} from './interfaces/definition';
import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeFlags, TNodeType, TViewNode} from './interfaces/node';
import {LQueries} from './interfaces/query';
import {RComment, RElement, Renderer3, isProceduralRenderer} from './interfaces/renderer';
import {CONTEXT, HOST_NODE, LView, QUERIES, RENDERER, TView} from './interfaces/view';
import {CONTAINER_INDEX, CONTEXT, HOST_NODE, LView, QUERIES, RENDERER, TView} from './interfaces/view';
import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
import {addRemoveViewFromContainer, appendChild, detachView, findComponentView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
import {getLView, getPreviousOrParentTNode} from './state';
@ -284,8 +284,9 @@ export function createContainerRef(
detach(index?: number): viewEngine_ViewRef|null {
const adjustedIdx = this._adjustIndex(index, -1);
detachView(this._lContainer, adjustedIdx, !!this._hostTNode.detached);
return this._viewRefs.splice(adjustedIdx, 1)[0] || null;
const view = detachView(this._lContainer, adjustedIdx, !!this._hostTNode.detached);
const wasDetached = this._viewRefs.splice(adjustedIdx, 1)[0] != null;
return wasDetached ? new ViewRef(view, view[CONTEXT], view[CONTAINER_INDEX]) : null;
}
private _adjustIndex(index?: number, shift: number = 0) {

14
packages/core/src/render3/view_ref.ts
View File

@ -258,11 +258,21 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
*/
checkNoChanges(): void { checkNoChanges(this.context); }
attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) { this._viewContainerRef = vcRef; }
attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) {
if (this._appRef) {
throw new Error('This view is already attached directly to the ApplicationRef!');
}
this._viewContainerRef = vcRef;
}
detachFromAppRef() { this._appRef = null; }
attachToAppRef(appRef: ApplicationRef) { this._appRef = appRef; }
attachToAppRef(appRef: ApplicationRef) {
if (this._viewContainerRef) {
throw new Error('This view is already attached to a ViewContainer!');
}
this._appRef = appRef;
}
private _lookUpContext(): T {
return this._context = this._lView[PARENT] ![this._componentIndex] as T;

34
packages/core/test/application_ref_spec.ts
View File

@ -429,25 +429,25 @@ class SomeComponent {
expect(appRef.viewCount).toBe(0);
});
fixmeIvy('unknown') &&
it('should not allow to attach a view to both, a view container and the ApplicationRef',
() => {
const comp = TestBed.createComponent(MyComp);
let hostView = comp.componentRef.hostView;
const containerComp = TestBed.createComponent(ContainerComp);
containerComp.detectChanges();
const vc = containerComp.componentInstance.vc;
const appRef: ApplicationRef = TestBed.get(ApplicationRef);
vc.insert(hostView);
expect(() => appRef.attachView(hostView))
.toThrowError('This view is already attached to a ViewContainer!');
hostView = vc.detach(0) !;
it('should not allow to attach a view to both, a view container and the ApplicationRef',
() => {
const comp = TestBed.createComponent(MyComp);
let hostView = comp.componentRef.hostView;
const containerComp = TestBed.createComponent(ContainerComp);
containerComp.detectChanges();
const vc = containerComp.componentInstance.vc;
const appRef: ApplicationRef = TestBed.get(ApplicationRef);
appRef.attachView(hostView);
expect(() => vc.insert(hostView))
.toThrowError('This view is already attached directly to the ApplicationRef!');
});
vc.insert(hostView);
expect(() => appRef.attachView(hostView))
.toThrowError('This view is already attached to a ViewContainer!');
hostView = vc.detach(0) !;
appRef.attachView(hostView);
expect(() => vc.insert(hostView))
.toThrowError('This view is already attached directly to the ApplicationRef!');
});
});
});