fix(ivy): ng-container with ViewContainerRef creates two comments (#30611)
With Ivy, injecting a `ViewContainerRef` for a `<ng-container>` element
results in two comments generated in the DOM. One comment as host
element for the `ElementContainer` and one for the generated `LContainer`
which is needed for the created `ViewContainerRef`.
This is problematic as developers expect the same anchor element for
the `LContainer` and the `ElementContainer` in order to be able to move
the host element of `<ng-container>` without leaving the actual
`LContainer` anchor element at the original location.
This worked differently in View Engine and various applications might
depend on the behavior where the `ViewContainerRef` shares the anchor
comment node with the host comment node of the `<ng-container>`. For
example `CdkTable` from `@angular/cdk` moves around the host element of
a `<ng-container>` and also expects embedded views to be inserted next
to the moved `<ng-container>` host element.
See: f8be5329f8/src/cdk/table/table.ts (L999-L1016)
Resolves FW-1341
PR Close #30611
			
			
This commit is contained in:
		
							parent
							
								
									e20b92ba37
								
							
						
					
					
						commit
						fa6cbb3ffe
					
				| @ -19,7 +19,7 @@ import {assertDefined, assertGreaterThan, assertLessThan} from '../util/assert'; | |||||||
| 
 | 
 | ||||||
| import {NodeInjector, getParentInjectorLocation} from './di'; | import {NodeInjector, getParentInjectorLocation} from './di'; | ||||||
| import {addToViewTree, createEmbeddedViewAndNode, createLContainer, renderEmbeddedTemplate} from './instructions/shared'; | import {addToViewTree, createEmbeddedViewAndNode, createLContainer, renderEmbeddedTemplate} from './instructions/shared'; | ||||||
| import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from './interfaces/container'; | import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer} from './interfaces/container'; | ||||||
| import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; | import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; | ||||||
| import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer'; | import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer'; | ||||||
| import {CONTEXT, LView, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view'; | import {CONTEXT, LView, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view'; | ||||||
| @ -29,7 +29,7 @@ import {getParentInjectorTNode} from './node_util'; | |||||||
| import {getLView, getPreviousOrParentTNode} from './state'; | import {getLView, getPreviousOrParentTNode} from './state'; | ||||||
| import {getParentInjectorView, hasParentInjector} from './util/injector_utils'; | import {getParentInjectorView, hasParentInjector} from './util/injector_utils'; | ||||||
| import {findComponentView} from './util/view_traversal_utils'; | import {findComponentView} from './util/view_traversal_utils'; | ||||||
| import {getComponentViewByIndex, getNativeByTNode, isComponent, isLContainer, isRootView, viewAttachedToContainer} from './util/view_utils'; | import {getComponentViewByIndex, getNativeByTNode, isComponent, isLContainer, isRootView, unwrapRNode, viewAttachedToContainer} from './util/view_utils'; | ||||||
| import {ViewRef} from './view_ref'; | import {ViewRef} from './view_ref'; | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| @ -313,8 +313,15 @@ export function createContainerRef( | |||||||
|     lContainer = slotValue; |     lContainer = slotValue; | ||||||
|     lContainer[ACTIVE_INDEX] = -1; |     lContainer[ACTIVE_INDEX] = -1; | ||||||
|   } else { |   } else { | ||||||
|     const commentNode = hostView[RENDERER].createComment(ngDevMode ? 'container' : ''); |     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.
 | ||||||
|  |     if (hostTNode.type === TNodeType.ElementContainer) { | ||||||
|  |       commentNode = unwrapRNode(slotValue) as RComment; | ||||||
|  |     } else { | ||||||
|       ngDevMode && ngDevMode.rendererCreateComment++; |       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
 |     // 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
 |     // can't use LTree to insert container's marker node (both parent of a comment node and the
 | ||||||
|  | |||||||
| @ -937,14 +937,14 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { | |||||||
|       fixture.detectChanges(); |       fixture.detectChanges(); | ||||||
|       expect(q.query.length).toEqual(1); |       expect(q.query.length).toEqual(1); | ||||||
|       expect(toHtml(fixture.nativeElement)) |       expect(toHtml(fixture.nativeElement)) | ||||||
|           .toEqual(`<div-query><!--ng-container-->Contenu<!--container--></div-query>`); |           .toEqual(`<div-query>Contenu<!--ng-container--></div-query>`); | ||||||
| 
 | 
 | ||||||
|       // Disable ng-if
 |       // Disable ng-if
 | ||||||
|       fixture.componentInstance.visible = false; |       fixture.componentInstance.visible = false; | ||||||
|       fixture.detectChanges(); |       fixture.detectChanges(); | ||||||
|       expect(q.query.length).toEqual(0); |       expect(q.query.length).toEqual(0); | ||||||
|       expect(toHtml(fixture.nativeElement)) |       expect(toHtml(fixture.nativeElement)) | ||||||
|           .toEqual(`<div-query><!--ng-container-->Contenu<!--container--></div-query>`); |           .toEqual(`<div-query>Contenu<!--ng-container--></div-query>`); | ||||||
|     }); |     }); | ||||||
|   }); |   }); | ||||||
| }); | }); | ||||||
|  | |||||||
| @ -61,6 +61,59 @@ describe('ViewContainerRef', () => { | |||||||
|          expect(fixture.componentInstance.foo).toBeAnInstanceOf(TemplateRef); |          expect(fixture.componentInstance.foo).toBeAnInstanceOf(TemplateRef); | ||||||
|        }); |        }); | ||||||
| 
 | 
 | ||||||
|  |     it('should use comment node of host ng-container as insertion marker', () => { | ||||||
|  |       @Component({template: 'hello'}) | ||||||
|  |       class HelloComp { | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       @NgModule({entryComponents: [HelloComp], declarations: [HelloComp]}) | ||||||
|  |       class HelloCompModule { | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       @Component({ | ||||||
|  |         template: ` | ||||||
|  |           <ng-container vcref></ng-container> | ||||||
|  |         ` | ||||||
|  |       }) | ||||||
|  |       class TestComp { | ||||||
|  |         @ViewChild(VCRefDirective, {static: true}) vcRefDir !: VCRefDirective; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       TestBed.configureTestingModule( | ||||||
|  |           {declarations: [TestComp, VCRefDirective], imports: [HelloCompModule]}); | ||||||
|  |       const fixture = TestBed.createComponent(TestComp); | ||||||
|  |       const {vcref, cfr, elementRef} = fixture.componentInstance.vcRefDir; | ||||||
|  |       fixture.detectChanges(); | ||||||
|  | 
 | ||||||
|  |       expect(fixture.nativeElement.innerHTML) | ||||||
|  |           .toMatch(/<!--(ng-container)?-->/, 'Expected only one comment node to be generated.'); | ||||||
|  | 
 | ||||||
|  |       const testParent = document.createElement('div'); | ||||||
|  |       testParent.appendChild(elementRef.nativeElement); | ||||||
|  | 
 | ||||||
|  |       expect(testParent.textContent).toBe(''); | ||||||
|  |       expect(testParent.childNodes.length).toBe(1); | ||||||
|  |       expect(testParent.childNodes[0].nodeType).toBe(Node.COMMENT_NODE); | ||||||
|  | 
 | ||||||
|  |       // Add a test component to the view container ref to ensure that
 | ||||||
|  |       // the "ng-container" comment was used as marker for the insertion.
 | ||||||
|  |       vcref.createComponent(cfr.resolveComponentFactory(HelloComp)); | ||||||
|  |       fixture.detectChanges(); | ||||||
|  | 
 | ||||||
|  |       expect(testParent.textContent).toBe('hello'); | ||||||
|  |       expect(testParent.childNodes.length).toBe(2); | ||||||
|  | 
 | ||||||
|  |       // With Ivy, views are inserted before the container comment marker.
 | ||||||
|  |       if (ivyEnabled) { | ||||||
|  |         expect(testParent.childNodes[0].nodeType).toBe(Node.ELEMENT_NODE); | ||||||
|  |         expect(testParent.childNodes[0].textContent).toBe('hello'); | ||||||
|  |         expect(testParent.childNodes[1].nodeType).toBe(Node.COMMENT_NODE); | ||||||
|  |       } else { | ||||||
|  |         expect(testParent.childNodes[0].nodeType).toBe(Node.COMMENT_NODE); | ||||||
|  |         expect(testParent.childNodes[1].nodeType).toBe(Node.ELEMENT_NODE); | ||||||
|  |         expect(testParent.childNodes[1].textContent).toBe('hello'); | ||||||
|  |       } | ||||||
|  |     }); | ||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|   describe('insert', () => { |   describe('insert', () => { | ||||||
| @ -1668,7 +1721,9 @@ class VCRefDirective { | |||||||
| 
 | 
 | ||||||
|   // Injecting the ViewContainerRef to create a dynamic container in which
 |   // Injecting the ViewContainerRef to create a dynamic container in which
 | ||||||
|   // embedded views will be created
 |   // embedded views will be created
 | ||||||
|   constructor(public vcref: ViewContainerRef, public cfr: ComponentFactoryResolver) {} |   constructor( | ||||||
|  |       public vcref: ViewContainerRef, public cfr: ComponentFactoryResolver, | ||||||
|  |       public elementRef: ElementRef) {} | ||||||
| 
 | 
 | ||||||
|   createView(s: string, index?: number): EmbeddedViewRef<any> { |   createView(s: string, index?: number): EmbeddedViewRef<any> { | ||||||
|     if (!this.tplRef) { |     if (!this.tplRef) { | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user