Skip to content

Commit 59a1cde

Browse files
m-absAlexander Vakrilov
authored and
Alexander Vakrilov
committed
Fix/ns views not cleaned on removal (#1740)
* fix(TemplatedItemsComponent): remove templates in ngOnDestroy() * fix(PageRouterOutlet): Destroy the activated componentRef on ngOnDestroy() * fix(NativeScriptRendererFactory): Empty the rootNgView when NativeScriptRendererFactory is destroyed * fix(ViewUtil): ViewUtil.removeChild() doesn't remove the child recursively * fix(NSLocationStrategy): remove outlets in ngOnDestroy() Make sure outlets and currentOutlet are removed when the NSLocationStrategy is destroyed. * fix: PageRouterOutlet.ngOnDestroy logged as NSLocationStrategy.ngOnDestroy * fix(ViewUtil): Removing FormattedString throw When removing the children from a Label and one of these children is a FormattedString, the FormattedTextProperty throw this exception: "TypeError: Cannot read property 'textTransform' of undefined" * fix: use ViewUtils to add Component to Page Without this the Page and componentView doesn't get the proper relation for then the page is destroyed. * fix: Remove all the children from the AppHostView * fix: ViewUtil.removeFromVisualTree logged under wrong name * fix(PageRouterOutlet): Don't remove grandchild views when moving component PageRouterOutlet.loadComponentInPage moves the componentView from its native parent, to the Page. Don't remove the componentView's children when it is removed from the native parent. * fix: ViewUtils tried to remote Span elements from Labels Just like with FormattedString this throws an exception * chor: skip detached elements for removeFromVisualTree
1 parent 9b144a4 commit 59a1cde

File tree

5 files changed

+67
-14
lines changed

5 files changed

+67
-14
lines changed

Diff for: nativescript-angular/directives/templated-items-comp.ts

+5
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ export abstract class TemplatedItemsComponent implements DoCheck, OnDestroy, Aft
100100

101101
ngOnDestroy() {
102102
this.templatedItemsView.off("itemLoading", this.onItemLoading, this);
103+
this.templatedItemsView = null;
104+
105+
if (this._templateMap) {
106+
this._templateMap.clear();
107+
}
103108
}
104109

105110
private setItemTemplates() {

Diff for: nativescript-angular/renderer.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ export class NativeScriptRendererFactory implements RendererFactory2 {
7373
this.componentRenderers.set(type.id, renderer);
7474
return renderer;
7575
}
76+
77+
ngOnDestroy(): void {
78+
if (isLogEnabled()) {
79+
traceLog(`NativeScriptRendererFactory.ngOnDestroy()`);
80+
}
81+
82+
while (this.rootNgView && this.rootNgView.firstChild) {
83+
this.viewUtil.removeChild(this.rootNgView, this.rootNgView.firstChild);
84+
}
85+
}
7686
}
7787

7888
export class NativeScriptRenderer extends Renderer2 {
@@ -129,7 +139,7 @@ export class NativeScriptRenderer extends Renderer2 {
129139
@profile
130140
parentNode(node: NgView): any {
131141
if (isLogEnabled()) {
132-
traceLog(`NativeScriptRenderer.parentNode for node: ${node}`);
142+
traceLog(`NativeScriptRenderer.parentNode for node: ${node} is ${node.parentNode}`);
133143
}
134144
return node.parentNode;
135145
}

Diff for: nativescript-angular/router/ns-location-strategy.ts

+9
Original file line numberDiff line numberDiff line change
@@ -744,4 +744,13 @@ export class NSLocationStrategy extends LocationStrategy {
744744
private getOutletKey(path: string, outletName: string): string {
745745
return path ? path + "-" + outletName : outletName;
746746
}
747+
748+
ngOnDestroy() {
749+
if (isLogEnabled()) {
750+
routerLog("NSLocationStrategy.ngOnDestroy()");
751+
}
752+
753+
this.outlets = [];
754+
this.currentOutlet = null;
755+
}
747756
}

Diff for: nativescript-angular/router/page-router-outlet.ts

+12-3
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,16 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
198198
});
199199
this.locationStrategy.clearOutlet(this.frame);
200200
} else {
201-
log("NSLocationStrategy.ngOnDestroy: no outlet available for page-router-outlet");
201+
log("PageRouterOutlet.ngOnDestroy: no outlet available for page-router-outlet");
202+
}
203+
204+
if (this.isActivated) {
205+
const c = this.activated.instance;
206+
this.activated.hostView.detach();
207+
destroyComponentRef(this.activated);
208+
209+
this.deactivateEvents.emit(c);
210+
this.activated = null;
202211
}
203212
}
204213

@@ -349,9 +358,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
349358
// Component loaded. Find its root native view.
350359
const componentView = componentRef.location.nativeElement;
351360
// Remove it from original native parent.
352-
this.viewUtil.removeChild(componentView.parent, componentView);
361+
this.viewUtil.removeChild(componentView.parent, componentView, false);
353362
// Add it to the new page
354-
page.content = componentView;
363+
this.viewUtil.insertChild(page, componentView);
355364

356365
const navigatedFromCallback = (<any>global).Zone.current.wrap((args: NavigatedData) => {
357366
if (args.isBackNavigation) {

Diff for: nativescript-angular/view-util.ts

+30-10
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,10 @@ export class ViewUtil {
157157
return next;
158158
}
159159

160-
public removeChild(parent: View, child: View) {
160+
public removeChild(parent: View, child: View, removeGrandchildren = true) {
161161
if (isLogEnabled()) {
162-
traceLog(`ViewUtil.removeChild parent: ${parent} child: ${child}`);
162+
traceLog(`ViewUtil.removeChild parent: ${parent} child: ${child} `
163+
+ `remove grandchildren: ${removeGrandchildren}`);
163164
}
164165

165166
if (!parent) {
@@ -169,8 +170,21 @@ export class ViewUtil {
169170
const extendedParent = this.ensureNgViewExtensions(parent);
170171
const extendedChild = this.ensureNgViewExtensions(child);
171172

173+
// Remove the child's children and their children
174+
// Unless called from PageRouterOutlet when the child is moved from once parent to another.
175+
while (extendedChild && extendedChild.firstChild && removeGrandchildren) {
176+
const grandchild = extendedChild.firstChild;
177+
if (isLogEnabled()) {
178+
traceLog(`ViewUtil.removeChild parent: ${parent} child: ${extendedChild} grandchild: ${grandchild}`);
179+
}
180+
181+
this.removeChild(extendedChild, grandchild);
182+
}
183+
172184
this.removeFromQueue(extendedParent, extendedChild);
173-
this.removeFromVisualTree(extendedParent, extendedChild);
185+
if (!isDetachedElement(extendedChild)) {
186+
this.removeFromVisualTree(extendedParent, extendedChild);
187+
}
174188
}
175189

176190
private removeFromQueue(parent: NgView, child: NgView) {
@@ -181,6 +195,7 @@ export class ViewUtil {
181195
if (parent.firstChild === child && parent.lastChild === child) {
182196
parent.firstChild = null;
183197
parent.lastChild = null;
198+
child.nextSibling = null;
184199
return;
185200
}
186201

@@ -196,6 +211,8 @@ export class ViewUtil {
196211
if (previous) {
197212
previous.nextSibling = child.nextSibling;
198213
}
214+
215+
child.nextSibling = null;
199216
}
200217

201218
// NOTE: This one is O(n) - use carefully
@@ -240,7 +257,7 @@ export class ViewUtil {
240257

241258
private removeFromVisualTree(parent: NgView, child: NgView) {
242259
if (isLogEnabled()) {
243-
traceLog(`ViewUtil.findPreviousElement parent: ${parent} child: ${child}`);
260+
traceLog(`ViewUtil.removeFromVisualTree parent: ${parent} child: ${child}`);
244261
}
245262

246263
if (parent.meta && parent.meta.removeChild) {
@@ -249,8 +266,6 @@ export class ViewUtil {
249266
this.removeLayoutChild(parent, child);
250267
} else if (isContentView(parent) && parent.content === child) {
251268
parent.content = null;
252-
parent.lastChild = null;
253-
parent.firstChild = null;
254269
} else if (isView(parent)) {
255270
parent._removeView(child);
256271
}
@@ -273,14 +288,15 @@ export class ViewUtil {
273288
}
274289

275290
public createView(name: string): NgView {
276-
if (isLogEnabled()) {
277-
traceLog(`Creating view: ${name}`);
278-
}
279-
291+
const originalName = name;
280292
if (!isKnownView(name)) {
281293
name = "ProxyViewContainer";
282294
}
283295

296+
if (isLogEnabled()) {
297+
traceLog(`Creating view: ${originalName} ${name}`);
298+
}
299+
284300
const viewClass = getViewClass(name);
285301
const view = <NgView>new viewClass();
286302
const ngView = this.setNgViewExtensions(view, name);
@@ -300,6 +316,10 @@ export class ViewUtil {
300316
}
301317

302318
private setNgViewExtensions(view: View, name: string): NgView {
319+
if (isLogEnabled()) {
320+
traceLog(`Make into a NgView view: ${view} name: "${name}"`);
321+
}
322+
303323
const ngView = view as NgView;
304324
ngView.nodeName = name;
305325
ngView.meta = getViewMeta(name);

0 commit comments

Comments
 (0)