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.

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

Nested can.Component with <content/> tag causes detached DOM nodes #1627

Closed
akagomez opened this Issue Apr 16, 2015 · 2 comments

Comments

Projects
None yet
4 participants
@akagomez
Contributor

akagomez commented Apr 16, 2015

@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Apr 17, 2015

Contributor

Incoming speculation, with links to lines in code:

When we replace the <content/> tag with the subtemplate, we never set replacements on the subtemplate nodeList when we register it. So when we remove the component from the DOM and unregister the subtemplate nodeList, since it doesn't have the replacements property, it doesn't get deleted from the nodeMap.

After doing some work arounds to force the contents we replace <content /> with to be seen as a parent element and, therefore, have that replacements array - I was able to remove your memory leak, but I'm sure we'll talk more in the morning.

Contributor

imjoshdean commented Apr 17, 2015

Incoming speculation, with links to lines in code:

When we replace the <content/> tag with the subtemplate, we never set replacements on the subtemplate nodeList when we register it. So when we remove the component from the DOM and unregister the subtemplate nodeList, since it doesn't have the replacements property, it doesn't get deleted from the nodeMap.

After doing some work arounds to force the contents we replace <content /> with to be seen as a parent element and, therefore, have that replacements array - I was able to remove your memory leak, but I'm sure we'll talk more in the morning.

daffl added a commit that referenced this issue Apr 20, 2015

daffl added a commit that referenced this issue Apr 20, 2015

daffl added a commit that referenced this issue Apr 20, 2015

@daffl daffl closed this in #1631 Apr 21, 2015

@daffl daffl added this to the 2.2.5 milestone Apr 21, 2015

daffl added a commit that referenced this issue May 1, 2015

@justinbmeyer justinbmeyer reopened this May 16, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 16, 2015

Contributor

This wasn't actually being tested. About to submit a fix.

Contributor

justinbmeyer commented May 16, 2015

This wasn't actually being tested. About to submit a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment