Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngSwitch): not leak when transcluded and cloned #1793

Closed
wants to merge 1 commit into from

Conversation

danilsomsikov
Copy link
Contributor

The leak can occur when ngSwich is used inside ngRepeat or any other
directive which does not attached transcluded content to DOM but clones
it.

Refactor ngSwitch to use controller instead of storing data on compile
node.

Closes #1621

The leak can occur when ngSwich is used inside ngRepeat or any other
directive which does not attached transcluded content to DOM but clones
it.

Refactor ngSwitch to use controller instead of storing data on compile
node.

Closes angular#1621
@petebacondarwin
Copy link
Contributor

@danilsomsikov Nice work. I'm afraid that the unit test that you have provided doesn't actually do anything.
Can you take a look at that?
In the mean while have you signed the CLA?

@danilsomsikov
Copy link
Contributor Author

It does. It reproduces situation when ngSwitch leaked. Te leak is being check at global afterEach in testabilityPatch.js

On 11.01.2013, at 22:36, Pete Bacon Darwin [email protected] wrote:

@danilsomsikov Nice work. I'm afraid that the unit test that you have provided doesn't actually do anything.
Can you take a look at that?
In the mean while have you signed the CLA?


Reply to this email directly or view it on GitHub.

@petebacondarwin
Copy link
Contributor

Ahah! Sorry my mistake!

On 11 January 2013 21:59, Danil Somsikov [email protected] wrote:

It does. It reproduces situation when ngSwitch leaked. Te leak is being
check at global afterEach in testabilityPatch.js

On 11.01.2013, at 22:36, Pete Bacon Darwin [email protected]
wrote:

@danilsomsikov Nice work. I'm afraid that the unit test that you have
provided doesn't actually do anything.
Can you take a look at that?
In the mean while have you signed the CLA?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1793#issuecomment-12165614.

@petebacondarwin
Copy link
Contributor

Jenkins build is good:
http://ci.angularjs.org/job/angular.js-pete/45/testReport/

On 11 January 2013 22:03, Peter Bacon Darwin [email protected] wrote:

Ahah! Sorry my mistake!

On 11 January 2013 21:59, Danil Somsikov [email protected] wrote:

It does. It reproduces situation when ngSwitch leaked. Te leak is being
check at global afterEach in testabilityPatch.js

On 11.01.2013, at 22:36, Pete Bacon Darwin [email protected]
wrote:

@danilsomsikov Nice work. I'm afraid that the unit test that you have
provided doesn't actually do anything.
Can you take a look at that?
In the mean while have you signed the CLA?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1793#issuecomment-12165614.

@danilsomsikov
Copy link
Contributor Author

I've signed the CLA.

@mhevery
Copy link
Contributor

mhevery commented Jan 18, 2013

MERGED

@mhevery mhevery closed this Jan 18, 2013
@mhevery mhevery reopened this Jan 18, 2013
@ghost ghost assigned IgorMinar Jan 18, 2013
@IgorMinar
Copy link
Contributor

this landed as a692dd8

please note that I changed the commit slightly. while the test and fix were correct the info about the cause of the leak in the commit message was not. the leak was caused by the switch element not being attached during the destruction of the parent dom (during which we do cleanup). it had nothing to do with cloning of dom nodes.

thanks for excellent work though!

@IgorMinar IgorMinar closed this Jan 18, 2013
@danilsomsikov
Copy link
Contributor Author

That's what I meant. Thanks for making it more clear!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ngSwitch directive leaks when inside transcluded content
4 participants