From d6bc9ab871490148f937f0587f2e9d16beca62ee Mon Sep 17 00:00:00 2001
From: Misko Hevery <misko@hevery.com>
Date: Thu, 6 Mar 2014 21:40:58 -0800
Subject: [PATCH] fix(change-detection): correctly process watch registration
 inside reaction FN.

Fix #678
---
 lib/change_detection/watch_group.dart       | 37 ++++++++++-----
 test/change_detection/watch_group_spec.dart | 23 ++++++++++
 test/core/scope_spec.dart                   | 51 +++++++++++++++++++--
 3 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/lib/change_detection/watch_group.dart b/lib/change_detection/watch_group.dart
index 421cec732..b3085562a 100644
--- a/lib/change_detection/watch_group.dart
+++ b/lib/change_detection/watch_group.dart
@@ -231,7 +231,13 @@ class WatchGroup implements _EvalWatchList, _WatchGroupList {
     // Must be done last
     _EvalWatchList._add(this, evalWatchRecord);
     _evalCost++;
-
+    if (_rootGroup.isInsideInvokeDirty) {
+      // This check means that we are inside invoke reaction function.
+      // Registering a new EvalWatch at this point will not run the
+      // .check() on it which means it will not be processed, but its
+      // reaction function will be run with null. So we process it manually.
+      evalWatchRecord.check();
+    }
     return evalWatchRecord;
   }
 
@@ -406,26 +412,33 @@ class RootWatchGroup extends WatchGroup {
     int count = 0;
     if (processStopwatch != null) processStopwatch.stop();
     Watch dirtyWatch = _dirtyWatchHead;
-    _dirtyWatchHead = _dirtyWatchTail = null;
+    _dirtyWatchHead = null;
     RootWatchGroup root = _rootGroup;
     root._removeCount = 0;
-    while(dirtyWatch != null) {
-      count++;
-      try {
-        if (root._removeCount == 0 || dirtyWatch._watchGroup.isAttached) {
-          dirtyWatch.invoke();
+    try {
+      while(dirtyWatch != null) {
+        count++;
+        try {
+          if (root._removeCount == 0 || dirtyWatch._watchGroup.isAttached) {
+            dirtyWatch.invoke();
+          }
+        } catch (e, s) {
+          if (exceptionHandler == null) rethrow; else exceptionHandler(e, s);
         }
-      } catch (e, s) {
-        if (exceptionHandler == null) rethrow; else exceptionHandler(e, s);
+        var nextDirtyWatch = dirtyWatch._nextDirtyWatch;
+        dirtyWatch._nextDirtyWatch = null;
+        dirtyWatch = nextDirtyWatch;
       }
-      var nextDirtyWatch = dirtyWatch._nextDirtyWatch;
-      dirtyWatch._nextDirtyWatch = null;
-      dirtyWatch = nextDirtyWatch;
+    } finally {
+      _dirtyWatchTail = null;
     }
     if (processStopwatch != null) processStopwatch..stop()..increment(count);
     return count;
   }
 
+  bool get isInsideInvokeDirty =>
+      _dirtyWatchHead == null && _dirtyWatchTail != null;
+
   /**
    * Add Watch into the asynchronous queue for later processing.
    */
diff --git a/test/change_detection/watch_group_spec.dart b/test/change_detection/watch_group_spec.dart
index 16c6cf174..b18aaa2d7 100644
--- a/test/change_detection/watch_group_spec.dart
+++ b/test/change_detection/watch_group_spec.dart
@@ -447,6 +447,29 @@ void main() {
         expect(logger).toEqual(['ABC']);
       });
 
+
+      it('should eval function eagerly when registered during reaction', () {
+        var fn = (arg) { logger('fn($arg)'); return arg; };
+        context['obj'] = {'fn': fn};
+        context['arg1'] = 'OUT';
+        context['arg2'] = 'IN';
+        var ast = new MethodAST(parse('obj'), 'fn', [parse('arg1')]);
+        var watch = watchGrp.watch(ast, (v, p) {
+          var ast = new MethodAST(parse('obj'), 'fn', [parse('arg2')]);
+          watchGrp.watch(ast, (v, p) {
+            logger('reaction: $v');
+          });
+        });
+
+        expect(logger).toEqual([]);
+        watchGrp.detectChanges();
+        expect(logger).toEqual(['fn(OUT)', 'fn(IN)', 'reaction: IN']);
+        logger.clear();
+        watchGrp.detectChanges();
+        expect(logger).toEqual(['fn(OUT)', 'fn(IN)']);
+      });
+
+
       it('should read connstant', () {
         // should fire on initial adding
         expect(watchGrp.fieldCost).toEqual(0);
diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart
index bc8b7da2f..090c87f29 100644
--- a/test/core/scope_spec.dart
+++ b/test/core/scope_spec.dart
@@ -1226,14 +1226,57 @@ void main() {
       it('should not trigger new watcher in the flush where it was added', inject((Scope scope) {
         var log = [] ;
         scope.context['foo'] = () => 'foo';
+        scope.context['name'] = 'misko';
+        scope.context['list'] = [2, 3];
+        scope.context['map'] = {'bar': 'chocolate'};
         scope.watch('1', (value, __) {
           expect(value).toEqual(1);
-          scope.watch('foo()', (value, __) {
-            log.add(value);
-          });
+          scope.watch('foo()', (value, __) => log.add(value));
+          scope.watch('name', (value, __) => log.add(value));
+          scope.watch('(foo() + "-" + name).toUpperCase()', (value, __) => log.add(value));
+          scope.watch('list', (value, __) => log.add(value));
+          scope.watch('map', (value, __) => log.add(value));
         });
         scope.apply();
-        expect(log).toEqual(['foo']);
+        expect(log).toEqual(['foo', 'misko', 'FOO-MISKO', [2, 3], {'bar': 'chocolate'}]);
+      }));
+
+
+      it('should allow multiple nested watches', inject((RootScope scope) {
+        scope.watch('1', (_, __) {
+          scope.watch('1', (_, __) {
+            scope.watch('1', (_, __) {
+              scope.watch('1', (_, __) {
+                scope.watch('1', (_, __) {
+                  scope.watch('1', (_, __) {
+                    scope.watch('1', (_, __) {
+                      scope.watch('1', (_, __) {
+                        scope.watch('1', (_, __) {
+                          scope.watch('1', (_, __) {
+                            scope.watch('1', (_, __) {
+                              scope.watch('1', (_, __) {
+                                scope.watch('1', (_, __) {
+                                  scope.watch('1', (_, __) {
+                                    scope.watch('1', (_, __) {
+                                      scope.watch('1', (_, __) {
+                                        // make this deeper then ScopeTTL;
+                                      });
+                                    });
+                                  });
+                                });
+                              });
+                            });
+                          });
+                        });
+                      });
+                    });
+                  });
+                });
+              });
+            });
+          });
+        });
+        expect(scope.apply).not.toThrow();
       }));