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

Commit d6bc9ab

Browse files
committed
fix(change-detection): correctly process watch registration inside reaction FN.
Fix #678
1 parent d7e77de commit d6bc9ab

File tree

3 files changed

+95
-16
lines changed

3 files changed

+95
-16
lines changed

lib/change_detection/watch_group.dart

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,13 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
231231
// Must be done last
232232
_EvalWatchList._add(this, evalWatchRecord);
233233
_evalCost++;
234-
234+
if (_rootGroup.isInsideInvokeDirty) {
235+
// This check means that we are inside invoke reaction function.
236+
// Registering a new EvalWatch at this point will not run the
237+
// .check() on it which means it will not be processed, but its
238+
// reaction function will be run with null. So we process it manually.
239+
evalWatchRecord.check();
240+
}
235241
return evalWatchRecord;
236242
}
237243

@@ -406,26 +412,33 @@ class RootWatchGroup extends WatchGroup {
406412
int count = 0;
407413
if (processStopwatch != null) processStopwatch.stop();
408414
Watch dirtyWatch = _dirtyWatchHead;
409-
_dirtyWatchHead = _dirtyWatchTail = null;
415+
_dirtyWatchHead = null;
410416
RootWatchGroup root = _rootGroup;
411417
root._removeCount = 0;
412-
while(dirtyWatch != null) {
413-
count++;
414-
try {
415-
if (root._removeCount == 0 || dirtyWatch._watchGroup.isAttached) {
416-
dirtyWatch.invoke();
418+
try {
419+
while(dirtyWatch != null) {
420+
count++;
421+
try {
422+
if (root._removeCount == 0 || dirtyWatch._watchGroup.isAttached) {
423+
dirtyWatch.invoke();
424+
}
425+
} catch (e, s) {
426+
if (exceptionHandler == null) rethrow; else exceptionHandler(e, s);
417427
}
418-
} catch (e, s) {
419-
if (exceptionHandler == null) rethrow; else exceptionHandler(e, s);
428+
var nextDirtyWatch = dirtyWatch._nextDirtyWatch;
429+
dirtyWatch._nextDirtyWatch = null;
430+
dirtyWatch = nextDirtyWatch;
420431
}
421-
var nextDirtyWatch = dirtyWatch._nextDirtyWatch;
422-
dirtyWatch._nextDirtyWatch = null;
423-
dirtyWatch = nextDirtyWatch;
432+
} finally {
433+
_dirtyWatchTail = null;
424434
}
425435
if (processStopwatch != null) processStopwatch..stop()..increment(count);
426436
return count;
427437
}
428438

439+
bool get isInsideInvokeDirty =>
440+
_dirtyWatchHead == null && _dirtyWatchTail != null;
441+
429442
/**
430443
* Add Watch into the asynchronous queue for later processing.
431444
*/

test/change_detection/watch_group_spec.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,29 @@ void main() {
447447
expect(logger).toEqual(['ABC']);
448448
});
449449

450+
451+
it('should eval function eagerly when registered during reaction', () {
452+
var fn = (arg) { logger('fn($arg)'); return arg; };
453+
context['obj'] = {'fn': fn};
454+
context['arg1'] = 'OUT';
455+
context['arg2'] = 'IN';
456+
var ast = new MethodAST(parse('obj'), 'fn', [parse('arg1')]);
457+
var watch = watchGrp.watch(ast, (v, p) {
458+
var ast = new MethodAST(parse('obj'), 'fn', [parse('arg2')]);
459+
watchGrp.watch(ast, (v, p) {
460+
logger('reaction: $v');
461+
});
462+
});
463+
464+
expect(logger).toEqual([]);
465+
watchGrp.detectChanges();
466+
expect(logger).toEqual(['fn(OUT)', 'fn(IN)', 'reaction: IN']);
467+
logger.clear();
468+
watchGrp.detectChanges();
469+
expect(logger).toEqual(['fn(OUT)', 'fn(IN)']);
470+
});
471+
472+
450473
it('should read connstant', () {
451474
// should fire on initial adding
452475
expect(watchGrp.fieldCost).toEqual(0);

test/core/scope_spec.dart

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,14 +1226,57 @@ void main() {
12261226
it('should not trigger new watcher in the flush where it was added', inject((Scope scope) {
12271227
var log = [] ;
12281228
scope.context['foo'] = () => 'foo';
1229+
scope.context['name'] = 'misko';
1230+
scope.context['list'] = [2, 3];
1231+
scope.context['map'] = {'bar': 'chocolate'};
12291232
scope.watch('1', (value, __) {
12301233
expect(value).toEqual(1);
1231-
scope.watch('foo()', (value, __) {
1232-
log.add(value);
1233-
});
1234+
scope.watch('foo()', (value, __) => log.add(value));
1235+
scope.watch('name', (value, __) => log.add(value));
1236+
scope.watch('(foo() + "-" + name).toUpperCase()', (value, __) => log.add(value));
1237+
scope.watch('list', (value, __) => log.add(value));
1238+
scope.watch('map', (value, __) => log.add(value));
12341239
});
12351240
scope.apply();
1236-
expect(log).toEqual(['foo']);
1241+
expect(log).toEqual(['foo', 'misko', 'FOO-MISKO', [2, 3], {'bar': 'chocolate'}]);
1242+
}));
1243+
1244+
1245+
it('should allow multiple nested watches', inject((RootScope scope) {
1246+
scope.watch('1', (_, __) {
1247+
scope.watch('1', (_, __) {
1248+
scope.watch('1', (_, __) {
1249+
scope.watch('1', (_, __) {
1250+
scope.watch('1', (_, __) {
1251+
scope.watch('1', (_, __) {
1252+
scope.watch('1', (_, __) {
1253+
scope.watch('1', (_, __) {
1254+
scope.watch('1', (_, __) {
1255+
scope.watch('1', (_, __) {
1256+
scope.watch('1', (_, __) {
1257+
scope.watch('1', (_, __) {
1258+
scope.watch('1', (_, __) {
1259+
scope.watch('1', (_, __) {
1260+
scope.watch('1', (_, __) {
1261+
scope.watch('1', (_, __) {
1262+
// make this deeper then ScopeTTL;
1263+
});
1264+
});
1265+
});
1266+
});
1267+
});
1268+
});
1269+
});
1270+
});
1271+
});
1272+
});
1273+
});
1274+
});
1275+
});
1276+
});
1277+
});
1278+
});
1279+
expect(scope.apply).not.toThrow();
12371280
}));
12381281

12391282

0 commit comments

Comments
 (0)