Skip to content

Commit 7e7747d

Browse files
committed
fix(WatchGroup): Handle watching elements of array that were removed.
Closes dart-archive#1046
1 parent d267af6 commit 7e7747d

File tree

3 files changed

+97
-4
lines changed

3 files changed

+97
-4
lines changed

lib/change_detection/watch_group.dart

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,18 +795,30 @@ class _EvalWatchRecord implements WatchRecord<_Handler> {
795795
}
796796

797797
bool check() {
798+
// A helper function used to apply another function. If the function happens to be fetching
799+
// from an array by index and there's a RangeError, the function will return _MaybeRemoved
800+
// instead of throwing an error.
801+
var applyFnHelper = (fn, args, namedArgs) {
802+
var value;
803+
try {
804+
return Function.apply(fn, args, namedArgs);
805+
} on RangeError catch(e) {
806+
if (args.length == 2 && args[0] is List && args[1] is num) return new _MaybeRemoved();
807+
else rethrow;
808+
}
809+
};
798810
var value;
799811
switch (mode) {
800812
case _MODE_MARKER_:
801813
case _MODE_NULL_:
802814
return false;
803815
case _MODE_PURE_FUNCTION_:
804816
if (!dirtyArgs) return false;
805-
value = Function.apply(fn, args, namedArgs);
817+
value = applyFnHelper(fn, args, namedArgs);
806818
dirtyArgs = false;
807819
break;
808820
case _MODE_FUNCTION_:
809-
value = Function.apply(fn, args, namedArgs);
821+
value = applyFnHelper(fn, args, namedArgs);
810822
dirtyArgs = false;
811823
break;
812824
case _MODE_PURE_FUNCTION_APPLY_:
@@ -873,3 +885,7 @@ class _EvalWatchRecord implements WatchRecord<_Handler> {
873885
return '${watchGrp.id}:${handler.expression}';
874886
}
875887
}
888+
889+
class _MaybeRemoved {
890+
toString() => "maybeRemoved";
891+
}

test/change_detection/watch_group_spec.dart

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,36 @@ void main() {
756756
expect(watchGrp.detectChanges()).not.toBe(null);
757757
expect(logger).toEqual([-3]);
758758
});
759+
760+
it('should return object of type _MaybeRemoved if closure is used to get an item by index '
761+
'and the index is out of range', () {
762+
context['a'] = ['item'];
763+
var ast = new ClosureAST( 'getItem', (obj, idx) => obj[idx], [parse('a'), parse('0')]);
764+
var watch = watchGrp.watch(ast, (v, p) => logger(v));
765+
766+
expect(watchGrp.detectChanges()).not.toBe(null);
767+
expect(logger).toEqual(['item']);
768+
logger.clear();
769+
770+
context['a'] = [];
771+
expect(watchGrp.detectChanges()).not.toBe(null);
772+
expect(logger.map((i) => '$i')).toEqual(['maybeRemoved']);
773+
});
774+
775+
it('should return object of type _MaybeRemoved if pure function is used to get an item by '
776+
'index and the index is out of range', () {
777+
context['a'] = ['item'];
778+
var ast = new PureFunctionAST( 'getItem', (obj, idx) => obj[idx], [parse('a'), parse('0')]);
779+
var watch = watchGrp.watch(ast, (v, p) => logger(v));
780+
781+
expect(watchGrp.detectChanges()).not.toBe(null);
782+
expect(logger).toEqual(['item']);
783+
logger.clear();
784+
785+
context['a'] = [];
786+
expect(watchGrp.detectChanges()).not.toBe(null);
787+
expect(logger.map((i) => '$i')).toEqual(['maybeRemoved']);
788+
});
759789
});
760790

761791
describe('evaluation', () {

test/directive/ng_repeat_spec.dart

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,6 @@ main() {
301301
expect(element.text).toEqual('misko:0|shyam:1|frodo:2|');
302302
});
303303

304-
305304
it(r'should expose iterator position as $first, $middle and $last when iterating over arrays',
306305
() {
307306
element = compile(
@@ -375,6 +374,55 @@ main() {
375374
expect(element.text).toEqual('a|b|Xc|d|X');
376375
});
377376

377+
describe('nested watching', () {
378+
it('should not throw RangeError when an item is being wathed and it is removed', () {
379+
element = compile(
380+
'<ul>'
381+
' <li ng-repeat="i in items">'
382+
' <input ng-model="items[\$index]">'
383+
' </li>'
384+
'</ul>');
385+
scope.context['items'] = ['misko', 'shyam', 'frodo'];
386+
scope.apply();
387+
expect(element.children.length).toEqual(3);
388+
scope.context['items'].remove('misko');
389+
scope.apply();
390+
expect(element.children.length).toEqual(2);
391+
});
392+
393+
it('should not throw RangeError when an item (somewhere in the middle of the list) is being '
394+
'watched and it is removed', () {
395+
element = compile(
396+
'<ul>'
397+
' <li ng-repeat="i in items">'
398+
' <input ng-model="items[\$index]">'
399+
' </li>'
400+
'</ul>');
401+
scope.context['items'] = ['misko', 'shyam', 'frodo', 'marko', 'pavel'];
402+
scope.apply();
403+
expect(element.children.length).toEqual(5);
404+
scope.context['items'].remove('shyam');
405+
scope.apply();
406+
expect(element.children.length).toEqual(4);
407+
});
408+
409+
it('should not throw RangeError when an items are being wathed and multiple items are removed '
410+
'at the same time', () {
411+
element = compile(
412+
'<ul>'
413+
' <li ng-repeat="i in items">'
414+
' <input ng-model="items[\$index]">'
415+
' </li>'
416+
'</ul>');
417+
scope.context['items'] = ['misko', 'shyam', 'frodo'];
418+
scope.apply();
419+
expect(element.children.length).toEqual(3);
420+
scope.context['items'].remove('misko');
421+
scope.context['items'].remove('frodo');
422+
scope.apply();
423+
expect(element.children.length).toEqual(1);
424+
});
425+
});
378426

379427
describe('stability', () {
380428
var a, b, c, d, lis;
@@ -394,7 +442,6 @@ main() {
394442
lis = element.querySelectorAll('li');
395443
});
396444

397-
398445
it(r'should preserve the order of elements', () {
399446
scope.context['items'] = [a, c, d];
400447
scope.apply();

0 commit comments

Comments
 (0)