Skip to content

fix: Persist the original "parentNode" when "retrieving" the root View created by createEmbeddedView #1542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 18, 2018
Merged

fix: Persist the original "parentNode" when "retrieving" the root View created by createEmbeddedView #1542

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fa40a2
fix: Persist the original "parentNode" when "retrieving" the root Vie…
VladimirAmiorkov Oct 4, 2018
04a2d87
Merge branch 'master' into amiorkov/embedded-views-memory-leak-fix
VladimirAmiorkov Oct 8, 2018
5baae6c
Merge branch 'master' into amiorkov/embedded-views-memory-leak-fix
SvetoslavTsenov Oct 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions nativescript-angular/element-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ export function getSingleViewRecursive(nodes: Array<any>, nestLevel: number): Vi

const parentLayout = rootLayout.parent;
if (parentLayout instanceof LayoutBase) {
let node = rootLayout.parentNode;
parentLayout.removeChild(rootLayout);
rootLayout.parentNode = node;

This comment was marked as abuse.

Sign in to view

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @NathanaelA ,

The reason for this fix is the following:
Setup:

  • In components like RadListView/ListView the component itself is going to handle the adding of the created templates to the UI tree
  • In those cases it is necessary to "remove" the view that is created by the Angular's createEmbeddedView because by design it automatically adds the created element to the UI tree
  • This is done via the getSingleViewRecursive

So here is the source of this leak:

  • The created "queue" of parent > child relationship that is created inside nativescript-angular initially (before getSingleViewRecursive removes the view) sets the parentNode to the actual ContainerViewRef that is calling the createEmbeddedView
  • Due to the above reason we have to remove that view from the UI tree which itself resets the parentNode to undefined because the elements (the template View) is removed from its parent
  • Later on when the GC is triggered that View does not have its correct "parentNode" (the one that was set when the creation of createEmbeddedView was called) and it cannot be correctly removed from the "queue".

So this fix makes sure that even if we "remove a view" from the UI by using the getSingleViewRecursive its "representation" in the internal elements queue will be correct and will be GC collectable.

}

return rootLayout;
Expand Down