-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(tab-link): avoid potential memory leak #1877
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// may not get called. See Angular issue #11606. | ||
ngOnDestroy() { | ||
super.ngOnDestroy(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find a good way of doing it and opted not to since the ripple didn't have a test for it either. Should I spend some time adding it to both places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would be worth it to have a test for both.
* Similarly to angular#1838, the `tab-link` destroy handler may not be called in certain situations, because it is being inherited from the MdRipple class. This PR explicitly calls the destroy handler. * Adds a unit test to make sure that the ripples are being cleaned up properly.
4aaa431
to
a999772
Compare
Updated with a unit test for the ripple and the tab link @jelbourn |
LGTM |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
tab-link
destroy handler may not be called in certain situations, because it is being inherited from the MdRipple class. This PR explicitly calls the destroy handler.