Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Commit 46b4c0e

Browse files
committed
fix(ng-repeat): handle the ref changing to null and back
This was broken in commit 09871cb. If the ng-repeat expression changed from an iterable to a null, ng-repeat would not notice it. This would cause further bugs as it would not reset its state and the ref became a collection again (i.e. once out of sync, it was gone forever.) Closes #1015
1 parent f7115aa commit 46b4c0e

File tree

2 files changed

+70
-35
lines changed

2 files changed

+70
-35
lines changed

Diff for: lib/directive/ng_repeat.dart

+43-35
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,7 @@ class NgRepeat {
132132
_watch = _scope.watch(
133133
_listExpr,
134134
(CollectionChangeRecord changes, _) {
135-
if (changes is! CollectionChangeRecord) return;
136-
_onChange(changes);
135+
_onChange((changes is CollectionChangeRecord) ? changes : null);
137136
},
138137
collection: true,
139138
formatters: formatters
@@ -142,7 +141,8 @@ class NgRepeat {
142141

143142
// Computes and executes DOM changes when the item list changes
144143
void _onChange(CollectionChangeRecord changes) {
145-
final int length = changes.length;
144+
final iterable = (changes == null) ? const [] : changes.iterable;
145+
final int length = (changes == null) ? 0 : changes.length;
146146
final rows = new List<_Row>(length);
147147
final changeFunctions = new List<Function>(length);
148148
final removedIndexes = <int>[];
@@ -170,43 +170,51 @@ class NgRepeat {
170170
_rows = new List<_Row>(length);
171171
for (var i = 0; i < length; i++) {
172172
changeFunctions[i] = (index, previousView) {
173-
addRow(index, changes.iterable.elementAt(i), previousView);
173+
addRow(index, iterable.elementAt(i), previousView);
174174
};
175175
}
176176
} else {
177-
changes.forEachRemoval((CollectionChangeItem removal) {
178-
var index = removal.previousIndex;
179-
var row = _rows[index];
180-
row.scope.destroy();
181-
_viewPort.remove(row.view);
182-
leftInDom.removeAt(domLength - 1 - index);
183-
});
177+
if (changes == null) {
178+
_rows.forEach((row) {
179+
row.scope.destroy();
180+
_viewPort.remove(row.view);
181+
});
182+
leftInDom.clear();
183+
} else {
184+
changes.forEachRemoval((CollectionChangeItem removal) {
185+
var index = removal.previousIndex;
186+
var row = _rows[index];
187+
row.scope.destroy();
188+
_viewPort.remove(row.view);
189+
leftInDom.removeAt(domLength - 1 - index);
190+
});
184191

185-
changes.forEachAddition((CollectionChangeItem addition) {
186-
changeFunctions[addition.currentIndex] = (index, previousView) {
187-
addRow(index, addition.item, previousView);
188-
};
189-
});
192+
changes.forEachAddition((CollectionChangeItem addition) {
193+
changeFunctions[addition.currentIndex] = (index, previousView) {
194+
addRow(index, addition.item, previousView);
195+
};
196+
});
190197

191-
changes.forEachMove((CollectionChangeItem move) {
192-
var previousIndex = move.previousIndex;
193-
var value = move.item;
194-
changeFunctions[move.currentIndex] = (index, previousView) {
195-
var previousRow = _rows[previousIndex];
196-
var childScope = previousRow.scope;
197-
var childContext = _updateContext(childScope.context, index, length);
198-
if (!identical(childScope.context[_valueIdentifier], value)) {
199-
childContext[_valueIdentifier] = value;
200-
}
201-
rows[index] = _rows[previousIndex];
202-
// Only move the DOM node when required
203-
if (domIndex < 0 || leftInDom[domIndex] != previousIndex) {
204-
_viewPort.move(previousRow.view, moveAfter: previousView);
205-
leftInDom.remove(previousIndex);
206-
}
207-
domIndex--;
208-
};
209-
});
198+
changes.forEachMove((CollectionChangeItem move) {
199+
var previousIndex = move.previousIndex;
200+
var value = move.item;
201+
changeFunctions[move.currentIndex] = (index, previousView) {
202+
var previousRow = _rows[previousIndex];
203+
var childScope = previousRow.scope;
204+
var childContext = _updateContext(childScope.context, index, length);
205+
if (!identical(childScope.context[_valueIdentifier], value)) {
206+
childContext[_valueIdentifier] = value;
207+
}
208+
rows[index] = _rows[previousIndex];
209+
// Only move the DOM node when required
210+
if (domIndex < 0 || leftInDom[domIndex] != previousIndex) {
211+
_viewPort.move(previousRow.view, moveAfter: previousView);
212+
leftInDom.remove(previousIndex);
213+
}
214+
domIndex--;
215+
};
216+
});
217+
}
210218
}
211219

212220
var previousView = null;

Diff for: test/directive/ng_repeat_spec.dart

+27
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,33 @@ main() {
107107
});
108108

109109

110+
it('should gracefully handle ref changing to null and back', () {
111+
scope.context['items'] = ['odin', 'dva',];
112+
element = compile(
113+
'<div>'
114+
'<ul>'
115+
'<li ng-repeat="item in items">{{item}};</li>'
116+
'</ul>'
117+
'</div>');
118+
scope.apply();
119+
expect(element.querySelectorAll('ul').length).toEqual(1);
120+
expect(element.querySelectorAll('li').length).toEqual(2);
121+
expect(element.text).toEqual('odin;dva;');
122+
123+
scope.context['items'] = null;
124+
scope.apply();
125+
expect(element.querySelectorAll('ul').length).toEqual(1);
126+
expect(element.querySelectorAll('li').length).toEqual(0);
127+
expect(element.text).toEqual('');
128+
129+
scope.context['items'] = ['odin', 'dva', 'tri'];
130+
scope.apply();
131+
expect(element.querySelectorAll('ul').length).toEqual(1);
132+
expect(element.querySelectorAll('li').length).toEqual(3);
133+
expect(element.text).toEqual('odin;dva;tri;');
134+
});
135+
136+
110137
it('should support formatters', () {
111138
element = compile(
112139
'<div><span ng-repeat="item in items | filter:myFilter">{{item}}</span></div>');

0 commit comments

Comments
 (0)