-
Notifications
You must be signed in to change notification settings - Fork 27.4k
refactor(ngTransclude): use transclusion function passed in to link rath... #5375
Conversation
CLA Signed under Daniel Tabuenca |
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. |
Signed again under different email address |
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). |
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.
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. 👍 |
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. |
LGTM also. @caitp Could you merge this into master? |
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
...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.