Skip to content

Commit

Permalink
chore(datepicker): unregister parent watchers on $destroy
Browse files Browse the repository at this point in the history 
Closes angular-ui#5242
  • Loading branch information
@deeg @wesleycho
deeg authored and wesleycho committed Jan 15, 2016
1 parent de24927 commit 16c641e
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 30 deletions.
70 changes: 41 additions & 29 deletions src/datepicker/datepicker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst
function($scope, $attrs, $parse, $interpolate, $log, dateFilter, datepickerConfig, $datepickerSuppressError, dateParser) {
var self = this,
ngModelCtrl = { $setViewValue: angular.noop }, // nullModelCtrl;
ngModelOptions = {};
ngModelOptions = {},
watchListeners = [];

// Modes chain
this.modes = ['day', 'month', 'year'];
Expand All @@ -44,24 +45,24 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst
// Watchable date attributes
angular.forEach(['minDate', 'maxDate'], function(key) {
if ($attrs[key]) {
$scope.$parent.$watch($attrs[key], function(value) {
watchListeners.push($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;
}
});

angular.forEach(['minMode', 'maxMode'], function(key) {
if ($attrs[key]) {
$scope.$parent.$watch($attrs[key], function(value) {
watchListeners.push($scope.$parent.$watch($attrs[key], function(value) {
self[key] = $scope[key] = angular.isDefined(value) ? value : $attrs[key];
if (key === 'minMode' && self.modes.indexOf($scope.datepickerMode) < self.modes.indexOf(self[key]) ||
key === 'maxMode' && self.modes.indexOf($scope.datepickerMode) > self.modes.indexOf(self[key])) {
$scope.datepickerMode = self[key];
}
});
}));
} else {
self[key] = $scope[key] = datepickerConfig[key] || null;
}
Expand All @@ -72,22 +73,22 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst

if (angular.isDefined($attrs.initDate)) {
this.activeDate = dateParser.fromTimezone($scope.$parent.$eval($attrs.initDate), ngModelOptions.timezone) || new Date();
$scope.$parent.$watch($attrs.initDate, function(initDate) {
watchListeners.push($scope.$parent.$watch($attrs.initDate, function(initDate) {
if (initDate && (ngModelCtrl.$isEmpty(ngModelCtrl.$modelValue) || ngModelCtrl.$invalid)) {
self.activeDate = dateParser.fromTimezone(initDate, ngModelOptions.timezone);
self.refreshView();
}
});
}));
} else {
this.activeDate = new Date();
}

$scope.disabled = angular.isDefined($attrs.disabled) || false;
if (angular.isDefined($attrs.ngDisabled)) {
$scope.$parent.$watch($attrs.ngDisabled, function(disabled) {
watchListeners.push($scope.$parent.$watch($attrs.ngDisabled, function(disabled) {
$scope.disabled = disabled;
self.refreshView();
});
}));
}

$scope.isActive = function(dateObject) {
Expand Down Expand Up @@ -248,6 +249,13 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst
self.refreshView();
}
};

$scope.$on("$destroy", function() {
//Clear all watch listeners on destroy
while (watchListeners.length) {
watchListeners.shift()();
}
});
}])

.controller('UibDaypickerController', ['$scope', '$element', 'dateFilter', function(scope, $element, dateFilter) {
Expand Down Expand Up @@ -573,12 +581,11 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst

.controller('UibDatepickerPopupController', ['$scope', '$element', '$attrs', '$compile', '$parse', '$document', '$rootScope', '$uibPosition', 'dateFilter', 'uibDateParser', 'uibDatepickerPopupConfig', '$timeout', 'uibDatepickerConfig',
function(scope, element, attrs, $compile, $parse, $document, $rootScope, $position, dateFilter, dateParser, datepickerPopupConfig, $timeout, datepickerConfig) {
var self = this;
var cache = {},
isHtml5DateInput = false;
var dateFormat, closeOnDateSelection, appendToBody, onOpenFocus,
datepickerPopupTemplateUrl, datepickerTemplateUrl, popupEl, datepickerEl,
ngModel, ngModelOptions, $popup, altInputFormats;
ngModel, ngModelOptions, $popup, altInputFormats, watchListeners = [];

scope.watchData = {};

Expand All @@ -600,17 +607,17 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi
} else {
dateFormat = attrs.uibDatepickerPopup || datepickerPopupConfig.datepickerPopup;
attrs.$observe('uibDatepickerPopup', function(value, oldValue) {
var newDateFormat = value || datepickerPopupConfig.datepickerPopup;
// Invalidate the $modelValue to ensure that formatters re-run
// FIXME: Refactor when PR is merged: https://github.com/angular/angular.js/pull/10764
if (newDateFormat !== dateFormat) {
dateFormat = newDateFormat;
ngModel.$modelValue = null;

if (!dateFormat) {
throw new Error('uibDatepickerPopup must have a date format specified.');
}
var newDateFormat = value || datepickerPopupConfig.datepickerPopup;
// Invalidate the $modelValue to ensure that formatters re-run
// FIXME: Refactor when PR is merged: https://github.com/angular/angular.js/pull/10764
if (newDateFormat !== dateFormat) {
dateFormat = newDateFormat;
ngModel.$modelValue = null;

if (!dateFormat) {
throw new Error('uibDatepickerPopup must have a date format specified.');
}
}
});
}

Expand Down Expand Up @@ -658,9 +665,9 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi

angular.forEach(['minMode', 'maxMode'], function(key) {
if (attrs[key]) {
scope.$parent.$watch(function() { return attrs[key]; }, function(value) {
watchListeners.push(scope.$parent.$watch(function() { return attrs[key]; }, function(value) {
scope.watchData[key] = value;
});
}));
datepickerEl.attr(cameltoDash(key), 'watchData.' + key);
}
});
Expand Down Expand Up @@ -692,13 +699,13 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi
if (attrs[key]) {
var getAttribute = $parse(attrs[key]);

scope.$parent.$watch(getAttribute, function(value) {
watchListeners.push(scope.$parent.$watch(getAttribute, function(value) {
if (key === 'minDate' || key === 'maxDate') {
cache[key] = angular.isDate(value) ? dateParser.fromTimezone(new Date(value), ngModelOptions.timezone) : new Date(dateFilter(value, 'medium'));
}

scope.watchData[key] = cache[key] || dateParser.fromTimezone(new Date(value), ngModelOptions.timezone);
});
}));

datepickerEl.attr(cameltoDash(key), 'watchData.' + key);
}
Expand Down Expand Up @@ -730,7 +737,7 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi
}
scope.date = dateParser.fromTimezone(value, ngModelOptions.timezone);
dateFormat = dateFormat.replace(/M!/, 'MM')
.replace(/d!/, 'dd');
.replace(/d!/, 'dd');

return dateFilter(scope.date, dateFormat);
});
Expand Down Expand Up @@ -770,6 +777,11 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi
$popup.remove();
element.unbind('keydown', inputKeydownBind);
$document.unbind('click', documentClickBind);

//Clear all watch listeners on destroy
while (watchListeners.length) {
watchListeners.shift()();
}
});
};

Expand All @@ -783,7 +795,7 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi
}

return scope.watchData.minDate && scope.compare(date, cache.minDate) < 0 ||
scope.watchData.maxDate && scope.compare(date, cache.maxDate) > 0;
scope.watchData.maxDate && scope.compare(date, cache.maxDate) > 0;
};

scope.compare = function(date1, date2) {
Expand Down Expand Up @@ -833,9 +845,9 @@ function(scope, element, attrs, $compile, $parse, $document, $rootScope, $positi

scope.disabled = angular.isDefined(attrs.disabled) || false;
if (attrs.ngDisabled) {
scope.$parent.$watch($parse(attrs.ngDisabled), function(disabled) {
watchListeners.push(scope.$parent.$watch($parse(attrs.ngDisabled), function(disabled) {
scope.disabled = disabled;
});
}));
}

scope.$watch('isOpen', function(value) {
Expand Down
56 changes: 55 additions & 1 deletion src/datepicker/test/datepicker.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,6 @@ describe('datepicker', function() {
expect(angular.element(button).prop('disabled')).toBe(false);
});
});

});

describe('`min-date` attribute', function () {
Expand Down Expand Up @@ -1203,6 +1202,38 @@ describe('datepicker', function() {
});
});

describe('gc', function() {
var datepickerScope;
beforeEach(function() {
$rootScope.minDate = new Date();
$rootScope.maxDate = new Date();
$rootScope.maxDate.setDate($rootScope.maxDate.getDate() + 1);
$rootScope.minMode = 'day';
$rootScope.maxMode = 'year';
$rootScope.initDate = new Date();
element = $compile('<uib-datepicker ng-model="date" min-date="minDate" max-date="maxDate" min-mode="minMode" max-mode="maxMode" init-date="initDate"></uib-datepicker>')($rootScope);
$rootScope.$digest();
datepickerScope = element.isolateScope();
});

it('should appropriately clean up $watch expressions', function() {
expect($rootScope.$$watchers.length).toBe(6);
['minDate', 'maxDate', 'minMode', 'maxMode', 'initDate'].forEach(function(prop) {
var $$watcher;
$rootScope.$$watchers.forEach(function($$watch) {
if ($$watch.exp === prop) {
$$watcher = $$watch;
}
});
expect(angular.isObject($$watcher)).toBe(true);
});

datepickerScope.$destroy();

expect($rootScope.$$watchers.length).toBe(1);
});
});

describe('setting datepickerConfig', function() {
var originalConfig = {};
beforeEach(inject(function(uibDatepickerConfig) {
Expand Down Expand Up @@ -2796,6 +2827,29 @@ describe('datepicker', function() {
});
});

describe('gc', function() {
var popupScope;
beforeEach(function() {
$rootScope.minDate = new Date();
$rootScope.maxDate = new Date();
$rootScope.maxDate.setDate($rootScope.maxDate.getDate() + 1);
$rootScope.minMode = 'day';
$rootScope.maxMode = 'year';
$rootScope.initDate = new Date();
element = $compile('<div><input ng-model="date" uib-datepicker-popup min-date="minDate" max-date="maxDate" min-mode="minMode" max-mode="maxMode" init-date="initDate"></uib-datepicker>')($rootScope);
$rootScope.$digest();
popupScope = element.find('input').isolateScope();
});

it('should appropriately clean up $watch expressions', function() {
expect($rootScope.$$watchers.length).toBe(6);

popupScope.$destroy();

expect($rootScope.$$watchers.length).toBe(1);
});
});

describe('with empty initial state', function() {
beforeEach(inject(function() {
$rootScope.date = null;
Expand Down

0 comments on commit 16c641e

Please sign in to comment.