Skip to content

Commit d4946e7

Browse files
damyanpetevdamyanpetev
authored
Merge pull request #15766 from IgniteUI/dpetev/elements-template-ref-cleanup
Elements template wrapper and content children cleanup fixes
2 parents 47bc779 + 0ea3413 commit d4946e7

File tree

2 files changed

+86
-56
lines changed

2 files changed

+86
-56
lines changed

‎projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ describe('Elements: ', () => {
5252
const gridComponent = (await gridEl.ngElementStrategy[ComponentRefKey]).instance as IgxGridComponent;
5353
const columnComponent = (await columnEl.ngElementStrategy[ComponentRefKey]).instance as IgxColumnComponent;
5454
expect(gridComponent.columnList.toArray()).toContain(columnComponent);
55+
56+
columnEl.remove();
57+
await firstValueFrom(timer(10 /* SCHEDULE_DELAY: DESTROY + QUERY */ * 3));
58+
expect(gridComponent.columnList.toArray()).toEqual([]);
5559
});
5660

5761
it(`should keep IgcNgElement instance in template of another IgcNgElement #15678`, async () => {

‎projects/igniteui-angular-elements/src/app/custom-strategy.ts

Lines changed: 82 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
2424
// public override componentRef: ComponentRef<any>|null = null;
2525

2626
protected element: IgcNgElement;
27+
/** The parent _component_'s element (a.k.a the semantic parent, rather than the DOM one after projection) */
28+
protected parentElement?: WeakRef<IgcNgElement>;
2729
/** Native Angular parent (if any) the Element is created under, usually as template of dynamic component (e.g. HGrid row island paginator) */
2830
protected angularParent: ComponentRef<any>;
2931
/** Cached child instances per query prop. Used for dynamic components's child templates that normally persist in Angular runtime */
@@ -41,14 +43,14 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
4143
*/
4244
public [ComponentRefKey] = new Promise<ComponentRef<any>>((resolve, _) => this.setComponentRef = resolve);
4345

44-
private _templateWrapper: TemplateWrapperComponent;
46+
private _templateWrapperRef: ComponentRef<TemplateWrapperComponent>;
4547
protected get templateWrapper(): TemplateWrapperComponent {
46-
if (!this._templateWrapper) {
48+
if (!this._templateWrapperRef) {
4749
const componentRef = (this as any).componentRef as ComponentRef<any>;
4850
const viewRef = componentRef.injector.get(ViewContainerRef);
49-
this._templateWrapper = viewRef.createComponent(TemplateWrapperComponent).instance;
51+
this._templateWrapperRef = viewRef.createComponent(TemplateWrapperComponent);
5052
}
51-
return this._templateWrapper;
53+
return this._templateWrapperRef.instance;
5254
}
5355

5456
private _configSelectors: string;
@@ -63,7 +65,7 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
6365
super(_componentFactory, _injector);
6466
}
6567

66-
override async initializeComponent(element: HTMLElement) {
68+
protected override async initializeComponent(element: HTMLElement) {
6769
if (!element.isConnected) {
6870
// D.P. 2022-09-20 do not initialize on connectedCallback that is not actually connected
6971
// connectedCallback may be called once your element is no longer connected
@@ -76,14 +78,13 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
7678
// TODO: Fail handling or cancellation needed?
7779
(this as any).componentRef = {};
7880

79-
const toBeOrphanedChildren = Array.from(element.children).filter(x => !this._componentFactory.ngContentSelectors.some(sel => x.matches(sel)));
80-
for (const iterator of toBeOrphanedChildren) {
81-
// TODO: special registration OR config for custom
82-
}
81+
// const toBeOrphanedChildren = Array.from(element.children).filter(x => !this._componentFactory.ngContentSelectors.some(sel => x.matches(sel)));
82+
// for (const iterator of toBeOrphanedChildren) {
83+
// // TODO: special registration OR config for custom
84+
// }
8385
let parentInjector: Injector;
8486
let parentAnchor: ViewContainerRef;
85-
const parents: IgcNgElement[] = [];
86-
let parentConfig: ComponentConfig;
87+
const parents: WeakRef<IgcNgElement>[] = [];
8788
const componentConfig = this.config?.find(x => x.component === this._componentFactory.componentType);
8889

8990
const configParents = componentConfig?.parents
@@ -98,11 +99,11 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
9899
reflectComponentType(x.component).selector
99100
]).join(','));
100101
if (node) {
101-
parents.push(node);
102+
parents.push(new WeakRef(node));
102103
}
103104
}
104105
// select closest of all possible config parents
105-
let parent = parents[0];
106+
let parent = parents[0]?.deref();
106107

107108
// Collected parents may include direct Angular HGrids, so only wait for configured parent elements:
108109
const configParent = configParents.find(x => x.selector === parent?.tagName.toLocaleLowerCase());
@@ -113,15 +114,16 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
113114
// ngElementStrategy getter is protected and also has initialization logic, though that should be safe at this point
114115
if (parent?.ngElementStrategy) {
115116
this.angularParent = parent.ngElementStrategy.angularParent;
116-
const parentComponentRef = await parent?.ngElementStrategy[ComponentRefKey];
117+
this.parentElement = new WeakRef(parent);
118+
let parentComponentRef = await parent?.ngElementStrategy[ComponentRefKey];
117119
parentInjector = parentComponentRef?.injector;
118120

119121
// TODO: Consider general solution (as in Parent w/ @igxAnchor tag)
120122
if (element.tagName.toLocaleLowerCase() === 'igc-grid-toolbar'
121123
|| element.tagName.toLocaleLowerCase() === 'igc-paginator') {
122124
// NOPE: viewcontainerRef will re-render this node again, no option for rootNode :S
123125
// this.componentRef = parentAnchor.createComponent(this.componentFactory.componentType, { projectableNodes, injector: childInjector });
124-
const parentComponentRef = await parent?.ngElementStrategy[ComponentRefKey];
126+
parentComponentRef = await parent?.ngElementStrategy[ComponentRefKey];
125127
parentAnchor = parentComponentRef?.instance.anchor;
126128
}
127129
} else if ((parent as any)?.__componentRef) {
@@ -182,44 +184,44 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
182184
// componentRef should also likely be protected:
183185
const componentRef = (this as any).componentRef as ComponentRef<any>;
184186

185-
for (let i = 0; i < parents.length; i++) {
186-
const parent = parents[i];
187-
188-
// find the respective config entry
189-
parentConfig = configParents.find(x => x.selector === parent?.tagName.toLocaleLowerCase());
190-
191-
if (!parentConfig) {
192-
continue;
193-
}
187+
const parentQueries = this.getParentContentQueries(componentConfig, parents, configParents);
194188

195-
const componentType = this._componentFactory.componentType;
196-
// TODO - look into more cases where query expects a certain base class but gets a subclass.
197-
// Related to https://github.com/IgniteUI/igniteui-angular/pull/12134#discussion_r983147259
198-
const contentQueries = parentConfig.contentQueries.filter(x => x.childType === componentType || x.childType === componentConfig.provideAs);
199-
200-
for (const query of contentQueries) {
201-
if (i > 0 && !query.descendants) {
202-
continue;
189+
for (const { parent, query } of parentQueries) {
190+
if (query.isQueryList) {
191+
parent.ngElementStrategy.scheduleQueryUpdate(query.property);
192+
if (this.angularParent) {
193+
// Cache the component in the parent (currently only paginator for HGrid),
194+
// so it is kept in the query even when detached from DOM
195+
this.addToParentCache(parent, query.property);
203196
}
204-
197+
} else {
205198
const parentRef = await parent.ngElementStrategy[ComponentRefKey];
206-
if (query.isQueryList) {
207-
parent.ngElementStrategy.scheduleQueryUpdate(query.property);
208-
if (this.angularParent) {
209-
// Cache the component in the parent (currently only paginator for HGrid),
210-
// so it is kept in the query even when detached from DOM
211-
this.addToParentCache(parent, query.property);
212-
}
213-
} else {
214-
parentRef.instance[query.property] = componentRef.instance;
215-
parentRef.changeDetectorRef.detectChanges();
216-
}
199+
parentRef.instance[query.property] = componentRef.instance;
200+
parentRef.changeDetectorRef.detectChanges();
217201
}
218202
}
219203

220204
if (['igc-grid', 'igc-tree-grid', 'igc-hierarchical-grid'].includes(element.tagName.toLocaleLowerCase())) {
221205
this.patchGridPopups();
222206
}
207+
208+
// instead of duplicating super.disconnect() w/ the scheduled destroy:
209+
componentRef.onDestroy(() => {
210+
if (this._templateWrapperRef) {
211+
this._templateWrapperRef.destroy();
212+
this._templateWrapperRef = null;
213+
}
214+
215+
// also schedule query updates on all parents:
216+
this.getParentContentQueries(componentConfig, parents, configParents)
217+
.filter(x => x.parent?.isConnected && x.query.isQueryList)
218+
.forEach(({ parent, query }) => {
219+
parent.ngElementStrategy.scheduleQueryUpdate(query.property);
220+
if (this.angularParent) {
221+
this.removeFromParentCache(parent, query.property);
222+
}
223+
});
224+
});
223225
}
224226

225227
public override setInputValue(property: string, value: any, transform?: (value: any) => any): void {
@@ -348,23 +350,47 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
348350
}
349351

350352
private addToParentCache(parentElement: IgcNgElement, queryName: string) {
351-
var cachedComponents = parentElement.ngElementStrategy.cachedChildComponents.get(queryName) || [];
353+
const cachedComponents = parentElement.ngElementStrategy.cachedChildComponents.get(queryName) || [];
352354
cachedComponents.push((this as any).componentRef.instance);
353355
parentElement.ngElementStrategy.cachedChildComponents.set(queryName, cachedComponents);
354356
}
355-
//#endregion schedule query update
356357

357-
/**
358-
* assignTemplateCallback
359-
*/
360-
public assignTemplateCallback(templateProp: string, callback: any) {
361-
const componentRef = (this as any).componentRef as ComponentRef<any>;
362-
if (componentRef) {
363-
const templateRef = this.templateWrapper.addTemplate(callback);
364-
componentRef.instance[templateProp] = templateRef;
365-
componentRef.changeDetectorRef.detectChanges();
358+
private removeFromParentCache(parentElement: IgcNgElement, queryName: string) {
359+
let cachedComponents = parentElement.ngElementStrategy.cachedChildComponents.get(queryName) || [];
360+
cachedComponents = cachedComponents.filter(x => x !== (this as any).componentRef.instance);
361+
parentElement.ngElementStrategy.cachedChildComponents.set(queryName, cachedComponents);
362+
}
363+
364+
/** Get all matching content questions from all parents */
365+
private getParentContentQueries(componentConfig: ComponentConfig, parents: WeakRef<IgcNgElement>[], configParents: ComponentConfig[]): { parent: IgcNgElement, query: ContentQueryMeta }[] {
366+
const queries: { parent: IgcNgElement, query: ContentQueryMeta }[] = [];
367+
368+
for (let i = 0; i < parents.length; i++) {
369+
const parent = parents[i]?.deref();
370+
371+
// find the respective config entry
372+
const parentConfig = configParents.find(x => x.selector === parent?.tagName.toLocaleLowerCase());
373+
if (!parentConfig) {
374+
continue;
375+
}
376+
377+
const componentType = this._componentFactory.componentType;
378+
// TODO - look into more cases where query expects a certain base class but gets a subclass.
379+
// Related to https://github.com/IgniteUI/igniteui-angular/pull/12134#discussion_r983147259
380+
const contentQueries = parentConfig.contentQueries.filter(x => x.childType === componentType || x.childType === componentConfig.provideAs);
381+
382+
for (const query of contentQueries) {
383+
if (i > 0 && !query.descendants) {
384+
continue;
385+
}
386+
queries.push({ parent, query });
387+
}
366388
}
389+
390+
return queries;
367391
}
392+
//#endregion schedule query update
393+
368394

369395
//#region Grid popups hide on scroll
370396
/**
@@ -400,7 +426,7 @@ class IgxCustomNgElementStrategy extends ComponentNgElementStrategy {
400426
}
401427
//#endregion
402428

403-
override disconnect(): void {
429+
public override disconnect(): void {
404430
if (this.angularParent) {
405431
this.angularParent.onDestroy(() => super.disconnect());
406432
} else {

0 commit comments

Comments
 (0)