Skip to content

Commit

Permalink
fix(ivy): properly insert views into ViewContainerRef injected by que…
Browse files Browse the repository at this point in the history
…rying <ng-container>

When asking for a ViewContainerRef on <ng-container> we do reuse <ng-container> comment
node as a LContainer's anachor. Before this fix the act of re-using a <ng-container>'s
comment node would result in this comment node being re-appended to the DOM in the wrong
place. With the fix in this PR we make sure that re-using <ng-container>'s comment node
doesn't result in unwanted DOM manipulation (ng-gontainer's comment node is already part
of the DOM and doesn't have to be re-created / re-appended).
  • Loading branch information
pkozlowski-opensource committed Nov 15, 2019
1 parent e51ec67 commit 43cf0eb
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 13 deletions.
29 changes: 16 additions & 13 deletions packages/core/src/render3/view_engine_compatibility.ts
Expand Up @@ -331,25 +331,28 @@ export function createContainerRef(
let commentNode: RComment;
// If the host is an element container, the native host element is guaranteed to be a
// comment and we can reuse that comment as anchor element for the new LContainer.
// The comment node in question is already part of the DOM structure so we don't need to append
// it again.
if (hostTNode.type === TNodeType.ElementContainer) {
commentNode = unwrapRNode(slotValue) as RComment;
} else {
ngDevMode && ngDevMode.rendererCreateComment++;
commentNode = hostView[RENDERER].createComment(ngDevMode ? 'container' : '');
}

// A container can be created on the root (topmost / bootstrapped) component and in this case we
// can't use LTree to insert container's marker node (both parent of a comment node and the
// commend node itself is located outside of elements hold by LTree). In this specific case we
// use low-level DOM manipulation to insert container's marker (comment) node.
if (isRootView(hostView)) {
const renderer = hostView[RENDERER];
const hostNative = getNativeByTNode(hostTNode, hostView) !;
const parentOfHostNative = nativeParentNode(renderer, hostNative);
nativeInsertBefore(
renderer, parentOfHostNative !, commentNode, nativeNextSibling(renderer, hostNative));
} else {
appendChild(commentNode, hostTNode, hostView);
// A `ViewContainerRef` can be injected by the root (topmost / bootstrapped) component. In
// this case we can't use TView / TNode data structures to insert container's marker node
// (both a parent of a comment node and the comment node itself are not part of any view). In
// this specific case we use low-level DOM manipulation to insert container's marker (comment)
// node.
if (isRootView(hostView)) {
const renderer = hostView[RENDERER];
const hostNative = getNativeByTNode(hostTNode, hostView) !;
const parentOfHostNative = nativeParentNode(renderer, hostNative);
nativeInsertBefore(
renderer, parentOfHostNative !, commentNode, nativeNextSibling(renderer, hostNative));
} else {
appendChild(commentNode, hostTNode, hostView);
}
}

hostView[hostTNode.index] = lContainer =
Expand Down
38 changes: 38 additions & 0 deletions packages/core/test/acceptance/view_insertion_spec.ts
Expand Up @@ -541,4 +541,42 @@ describe('view insertion', () => {
});
});

describe('non-regression', () => {

// https://github.com/angular/angular/issues/33679
it('should insert views into ViewContainerRef injected by querying <ng-container>', () => {

@Component({
selector: 'app-root',
template: `
<div>container start|</div>
<ng-container #container></ng-container>
<div>|container end</div>
<ng-template #template >test</ng-template>
<div (click)="click()" >|click</div>
`
})
class AppComponent {
@ViewChild('container', {read: ViewContainerRef, static: true})
vcr !: ViewContainerRef;

@ViewChild('template', {read: TemplateRef, static: true}) template !: TemplateRef<any>;

click() { this.vcr.createEmbeddedView(this.template, undefined, 0); }
}

TestBed.configureTestingModule({
declarations: [AppComponent],
});
const fixture = TestBed.createComponent(AppComponent);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('container start||container end|click');

fixture.componentInstance.click();
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toBe('container start|test|container end|click');
});

});
});

0 comments on commit 43cf0eb

Please sign in to comment.