Skip to content

Commit

Permalink
feat(watch_group): split group manipulation into newGroup and addGroup
Browse files Browse the repository at this point in the history 
Previously, creating a new watch group simultaneously attached it to the parent from which it was created. Now, we have split this behavior into two steps so that it is possible to create a watch group, set up watches and then choose which parent to attach it to at a later point in time. You can also remove a watch group and attach it to a different parent. These features allow a more elegant mechanism for turning watches on/off based on whether or not the associated view is attached to the DOM. A primary use case for this is to tie into the attachedCallback/detachedCallback of the custom element lifecycle in order to better manage databinding present in element templates.

Closes issue angular#32
  • Loading branch information
@EisenbergEffect @caitp
EisenbergEffect authored and caitp committed May 29, 2014
1 parent 6364b5a commit 901e821
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 41 deletions.
65 changes: 38 additions & 27 deletions src/watch_group.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,14 @@ function putIfAbsent(obj, key, ctor) {
}

export class WatchGroup {
constructor(parentWatchGroup, getterCache, context, cache, rootGroup) {
// TODO: Traceur Assertions
// assert(parentWatchGroup is WatchGroup)
// assert(changeDetector is ChangeDetector)
// assert(context and context is Function or Object)
// assert(rootGroup is RootWatchGroup)
this._parentWatchGroup = parentWatchGroup;
constructor(id, getterCache, context, cache, rootGroup) {
this.id = id;

// Initialize _WatchGroupList
this._watchGroupHead = this._watchGroupTail = null;
this._nextWatchGroup = this._prevWatchGroup = null;
this.id = `${parentWatchGroup.id}.${parentWatchGroup._nextChildId++}`;
this._dirtyWatchHead = this._dirtyWatchTail = null;

this._getterCache = getterCache;
this.context = context;
this._cache = cache;
Expand All @@ -54,8 +51,7 @@ export class WatchGroup {
this._evalWatchHead = this._evalWatchTail = this._marker;

this._dirtyMarker = ChangeRecord.marker();
this._recordTail = this._parentWatchGroup._childInclRecordTail;
this._recordHead = this._recordTail = this._recordAdd(this._dirtyMarker);
this._recordHead = this._recordTail = this._dirtyMarker;

// Stats...
this.fieldCost = 0; // Stats: Number of field watchers which are in use
Expand Down Expand Up @@ -179,51 +175,66 @@ export class WatchGroup {
// [context]. If not present than child expressions will evaluate on same context allowing
// the reuse of the expression cache.
newGroup(context) {
var prev = this._childWatchGroupTail._evalWatchTail;
var next = prev._nextEvalWatch;

if (arguments.length === 0 || context === null) {
context = this.context;
}

var root = this._rootGroup === null ? this : this._rootGroup;
var id = `${this.id}.${this._nextChildId++}`;
var cache = context === null ? this._cache : {};

var childGroup = new WatchGroup(this, this._getterCache, context, cache, root);
var childGroup = new WatchGroup(id, this._getterCache, context, cache, root);

return childGroup;
}

addGroup(childGroup){
childGroup.id = `${this.id}.${this._nextChildId++}`;
childGroup._parentWatchGroup = this;

var prevEval = this._childWatchGroupTail._evalWatchTail;
var nextEval = prevEval._nextEvalWatch;
var prevRecord = this._childWatchGroupTail._recordTail;
var nextRecord = prevRecord.nextRecord;

_WatchGroupList._add(this, childGroup);

var marker = childGroup._marker;
var evalMarker = childGroup._marker;

marker._prevEvalWatch = prev;
marker._nextEvalWatch = next;
evalMarker._prevEvalWatch = prevEval;
evalMarker._nextEvalWatch = nextEval;

if (prev !== null) prev._nextEvalWatch = marker;
if (next !== null) next._prevEvalWatch = marker;
if (prevEval !== null) prevEval._nextEvalWatch = evalMarker;
if (nextEval !== null) nextEval._prevEvalWatch = evalMarker;

return childGroup;
var childRecordHead = childGroup._recordHead;
var childRecordTail = childGroup._recordTail;

childRecordHead.prevRecord = prevRecord;
childRecordTail.nextRecord = nextRecord;

if (prevRecord !== null) prevRecord.nextRecord = childRecordHead;
if (nextRecord !== null) nextRecord.prevRecord = childRecordTail;

// TODO:(eisenbergeffect) attach observers associated with dirty records?
}

// Remove/destroy [WatchGroup] and all of its watches
remove() {
// TODO:(misko) This code is not right.
// 1) It fails to release [ChangeDetector] [WatchRecord]s
// TODO:(eisenbergeffect) detach observers associated with dirty records?

var prevRecord = this._recordHead.prevRecord;
var nextRecord = this._childInclRecordTail.nextRecord;

if (prevRecord !== null) prevRecord.nextRecord = nextRecord;
if (nextRecord !== null) nextRecord.prevRecord = prevRecord;

// TODO:(eisenbergeffect) investigate these two lines; not sure such properties exist on records
this._recordHead._prevWatchGroup = null;
this._recordTail._prevWatchGroup = null;
this._recordHead = this._recordTail = null;


_WatchGroupList._remove(this._parentWatchGroup, this);
this._nextWatchGroup = this._prevWatchGroup = null;

//TODO: this._changeDetector.remove();

this._rootGroup._removeCount++;
this._parentWatchGroup = null;

Expand Down
21 changes: 14 additions & 7 deletions test/dirtychecking.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ describe('DirtyCheckingChangeDetector', function() {
setup();
var obj = {};
var ra = watchGrp.watchField(obj, 'a', '0a');
var child1a = watchGrp.newGroup();
var child1b = watchGrp.newGroup();
var child2 = child1a.newGroup();
var child1a = createAndAddGroup(watchGrp);
var child1b = createAndAddGroup(watchGrp);
var child2 = createAndAddGroup(child1a);

child1a.watchField(obj, 'a', '1a');
child1b.watchField(obj, 'a', '1b');
Expand All @@ -153,7 +153,7 @@ describe('DirtyCheckingChangeDetector', function() {
setup();
var obj = {};
var ra = watchGrp.watchField(obj, 'a', 'a');
var child = watchGrp.newGroup();
var child = createAndAddGroup(watchGrp);
var cb = child.watchField(obj, 'b', 'b');

obj['a'] = obj['b'] = 1;
Expand All @@ -177,9 +177,9 @@ describe('DirtyCheckingChangeDetector', function() {

it('should properly add children', function() {
setup();
var a = watchGrp.newGroup();
var aChild = a.newGroup();
var b = watchGrp.newGroup();
var a = createAndAddGroup(watchGrp);
var aChild = createAndAddGroup(a);
var b = createAndAddGroup(watchGrp);
expect(function() {
watchGrp.collectChanges();
}).not.toThrow();
Expand Down Expand Up @@ -580,3 +580,10 @@ class _User {
this.age = age;
}
}

function createAndAddGroup(parentGroup, context){
var childGroup = parentGroup.newGroup(context);
parentGroup.addGroup(childGroup);
return childGroup;
}

9 changes: 8 additions & 1 deletion test/observer.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('observer', function() {
group;

setup(observer);
group = watchGrp.newGroup();
group = createAndAddGroup(watchGrp);
group.watchField(user, 'name', null);

expect(observer.closeCalls).toBe(0);
Expand Down Expand Up @@ -183,3 +183,10 @@ class ExplicitObserver{
this.callback = null;
}
}

function createAndAddGroup(parentGroup, context){
var childGroup = parentGroup.newGroup(context);
parentGroup.addGroup(childGroup);
return childGroup;
}

112 changes: 106 additions & 6 deletions test/watchgroup.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ describe('WatchGroup', function() {

it('should react when pure function return value changes in child watchGroup', function() {
setup();
var childWatchGrp = watchGrp.newGroup({'a': 1, 'b': 2});
var childWatchGrp = createAndAddGroup(watchGrp, {'a': 1, 'b': 2});

var watch = childWatchGrp.watchExpression(new PureFunctionAST('add', function(a, b) {
logger.log('+');
Expand Down Expand Up @@ -707,9 +707,9 @@ describe('WatchGroup', function() {
proxy1 = Object.create(context);
proxy2 = Object.create(context);
proxy3 = Object.create(context);
child1a = watchGrp.newGroup(proxy1);
child1b = watchGrp.newGroup(proxy2);
child2 = child1a.newGroup(proxy3);
child1a = createAndAddGroup(watchGrp, proxy1);
child1b = createAndAddGroup(watchGrp, proxy2);
child2 = createAndAddGroup(child1a, proxy3);

child1a.watchExpression(parse('a'), logValue('1a'));
child1b.watchExpression(parse('a'), logValue('1b'));
Expand All @@ -727,6 +727,98 @@ describe('WatchGroup', function() {
proxy1 = proxy2 = proxy3 = child1a = child1b = child2 = null;
});

it('should be able to add watches without being attached to a parent', () => {
var changes = 0,
context = {a:'something'};

setup();
child1a = watchGrp.newGroup(context);
child1a.watchExpression(parse('a'), logValue('1a'));

context.a = 'another';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(0);
expect(changes).toBe(0);
expect(`${logger}`).toBe('1a');
});

it('should be able to connect a group with existing watches', () => {
var changes = 0,
context = {a:'something'};

setup();
child1a = watchGrp.newGroup(context);
child1a.watchExpression(parse('a'), logValue('1a'));
watchGrp.addGroup(child1a);

context.a = 'another';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(1);
expect(changes).toBe(1);
expect(`${logger}`).toBe('1a');
});

it('should be able to re-connect a group with existing watches', () => {
var changes = 0,
context = {a:'something'};

setup();
child1a = watchGrp.newGroup(context);
child1a.watchExpression(parse('a'), logValue('1a'));

watchGrp.addGroup(child1a);
child1a.remove();

context.a = 'another';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(0);
expect(changes).toBe(0);

watchGrp.addGroup(child1a);

context.a = 'another2';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(1);
expect(changes).toBe(1);
});

it('should be able to move a group with existing watches', () => {
var changes = 0,
context = {a:'something'};

setup();
child1a = watchGrp.newGroup(context);
child1a.watchExpression(parse('a'), logValue('1a'));

watchGrp.addGroup(child1a);
child1a.remove();

context.a = 'another';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(0);
expect(changes).toBe(0);

child2 = watchGrp.newGroup({});
watchGrp.addGroup(child2);
child2.addGroup(child1a);

context.a = 'another2';
watchGrp.detectChanges(null, () => changes++);

expect(child1a.fieldCost).toBe(1);
expect(watchGrp.totalFieldCost).toBe(1);
expect(changes).toBe(1);
});

it('should set field cost to expected value', function() {
setupChildGroups();
Expand Down Expand Up @@ -802,7 +894,7 @@ describe('WatchGroup', function() {
it('should not call reaction function on removed group', function() {
setup({ 'name': 'misko' });

var child = watchGrp.newGroup(context);
var child = createAndAddGroup(watchGrp, context);
watchGrp.watchExpression(parse('name'), function(v, p) {
logger.log(`root ${v}`);
if (v === 'destroy') {
Expand Down Expand Up @@ -831,7 +923,8 @@ describe('WatchGroup', function() {
watchGrp.watchExpression(parse('a'), function(v, p) {
logger.log(v);
});
watchGrp.newGroup(childContext).watchExpression(parse('b'), logCurrentValue);

createAndAddGroup(watchGrp, childContext).watchExpression(parse('b'), logCurrentValue);

watchGrp.detectChanges();
expect(logger.toArray()).toEqual(['OK', 'OK']);
Expand All @@ -846,3 +939,10 @@ describe('WatchGroup', function() {
});
});
});

function createAndAddGroup(parentGroup, context){
var childGroup = parentGroup.newGroup(context);
parentGroup.addGroup(childGroup);
return childGroup;
}

0 comments on commit 901e821

Please sign in to comment.