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

refactor(ngTransclude): use transclusion function passed in to link rath... #5375

Closed
wants to merge 1 commit into from

Conversation

dtabuenc
Copy link
Contributor

...er than controller

Since we now pass in the transclusion function directly to the link function, we no longer need
the old scheme whereby we saved the transclude function injected into the controller for later
use in during linking.

This commit removes the controller and simplifies ngTransclude.

@dtabuenc
Copy link
Contributor Author

CLA Signed under Daniel Tabuenca

@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@dtabuenc
Copy link
Contributor Author

Signed again under different email address

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@ghost ghost assigned tbosch Dec 13, 2013
@OlivierPerceboisGarve
Copy link

I'd be happy to see triage comment on this. ( because I recently broke my teeth on related stuff ;( )

Since we now pass in the transclusion function directly to the link function, we no longer need
the old scheme whereby we saved the transclude function injected into the controller for later
use in during linking.

This commit removes the controller and simplifies ngTransclude.
@renz45
Copy link

renz45 commented Feb 7, 2014

I should have looked through issues a bit closer before submitting my own pull request. I've determined that this fixes a memory leak caused by ngTransclude: #6181

It should be bumped up in priority me thinks. 👍

@caitp
Copy link
Contributor

caitp commented Feb 11, 2014

LGTM, I think this will get another pair of eyes before it lands. It's really unfortunate that there's no good way to test this, but it's nice that it shrinks code a tiny bit.

@tbosch
Copy link
Contributor

tbosch commented Feb 11, 2014

LGTM also. @caitp Could you merge this into master?

@caitp caitp closed this in 08793a6 Feb 11, 2014
@caitp
Copy link
Contributor

caitp commented Feb 11, 2014

Merged, thanks a bunch @renz45 and @dtabuenc

khepin pushed a commit to khepin/angular.js that referenced this pull request Feb 19, 2014
Since we now pass in the transclusion function directly to the link function, we no longer need
the old scheme whereby we saved the transclude function injected into the controller for later
use in during linking.

Additionally, this change may aid in correcting a memory leak of detached DOM nodes (see angular#6181
for details).

This commit removes the controller and simplifies ngTransclude.

Closes angular#5375
Closes angular#6181
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants