Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Unregister watches on the parent scope #5242

Closed
pedroclayman opened this issue Jan 14, 2016 · 3 comments
Closed

Unregister watches on the parent scope #5242

pedroclayman opened this issue Jan 14, 2016 · 3 comments

Comments

@pedroclayman
Copy link

Several watches are registered on the parent scope in the datepicker.js. Those watches are going to be unregistered the moment the parent scope will be destroyed. Wouldn't it be wiser to unregister them when the directive's scope is being destroyed?

  angular.forEach(['minDate', 'maxDate'], function(key) {
    if ($attrs[key]) {
      $scope.$parent.$watch($attrs[key], function(value) {
        self[key] = value ? angular.isDate(value) ? dateParser.fromTimezone(new Date(value), ngModelOptions.timezone) : new Date(dateFilter(value, 'medium')) : null;
        self.refreshView();
      });
    } else {
      self[key] = datepickerConfig[key] ? dateParser.fromTimezone(new Date(datepickerConfig[key]), ngModelOptions.timezone) : null;
    }
  });
@wesleycho
Copy link
Contributor

I was wondering about watchers on the parent throughout several components - they probably should be garbage collected on $destroy.

Should be an easy PR (or few PRs for the other components where this is happening) if you'd like to try your hand at it.

@deeg
Copy link
Contributor

deeg commented Jan 14, 2016

I will try to get to this tonight or tomorrow.

deeg added a commit to deeg/bootstrap that referenced this issue Jan 15, 2016
deeg added a commit to deeg/bootstrap that referenced this issue Jan 15, 2016
deeg added a commit to deeg/bootstrap that referenced this issue Jan 15, 2016
wesleycho pushed a commit to wesleycho/bootstrap that referenced this issue Jan 15, 2016
wesleycho pushed a commit to wesleycho/bootstrap that referenced this issue Jan 15, 2016
wesleycho pushed a commit to wesleycho/bootstrap that referenced this issue Jan 16, 2016
wesleycho pushed a commit to wesleycho/bootstrap that referenced this issue Jan 16, 2016
@pedroclayman
Copy link
Author

thanks!

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

No branches or pull requests

3 participants