Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add pushToStack() and popFromStack() methods to allow async Observations #52

Closed
wants to merge 2 commits into from
Closed

Add pushToStack() and popFromStack() methods to allow async Observations #52

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c159ee6
Add pushToStack() and popFromStack() methods to allow async Observations
BigAB Feb 6, 2017
6d23e53
Remove ES6 syntax, move assignValue function outside of Observation c…
BigAB Feb 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions can-observation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ function Observation(func, context, compute){
this.childDepths = {};
this.ignore = 0;
this.needsUpdate= false;
if (!func) {
this.start = function(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is start needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this function need to be re-generated? Could only one be created and just use a variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

start is needed because it is called in update(), and if I modified update to not call start(), I wasn't getting updates. There may be a better way to do this though, I am open to suggestion.

I'll declare the function outside of the constructor and reuse it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to inserting the start method there, could be modifying update() to be:

	update: function(batchNum){
		if(this.needsUpdate) {
			remaining.updates--;
		}
		this.needsUpdate = false;
		if(this.bound) {
			// Keep the old value.
			var oldValue = this.value;
			this.oldValue = null;
			// Get the new value and register this event handler to any new observables.
			if (this.func) {
				this.start();
			}
			if(oldValue !== this.value || !this.func) {
				this.compute.updater(this.value, oldValue, batchNum);
				return true;
			}
		}
	},

Would that be a more preferable approach?

this.value = {}; // always set a unique value, so there is always an update
};
}
}

// ### observationStack
Expand Down Expand Up @@ -268,6 +273,28 @@ assign(Observation.prototype,{
this.removeEdge(ob);
}
this.newObserved = {};
},
/**
* @function can-observation.prototype.pushToStack pushToStack
* @parent can-observation.prototype prototype
*
* @signature `observation.pushToStack()`
*
* Starts observing all observables in general until `.popFromStack()` is called.
*
*/
pushToStack() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ES5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, sorry, force of habit

this.bound = true;
this.oldObserved = this.newObserved || {};
this.ignore = 0;
this.newObserved = {};
// Add this function call's observation to the stack
observationStack.push(this);
},
popFromStack() {
// pops off the observation, and returns it.
observationStack.pop();
this.updateBindings();
}
/**
* @property {*} can-observation.prototype.value
Expand Down
25 changes: 25 additions & 0 deletions can-observation_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,28 @@ QUnit.test('should throw if can-namespace.Observation is already defined', funct
start();
});
});

QUnit.test('pushToStack and popToStack allow observing reads outside of a function', function() {
stop(3);
var expected = ['Adam', 'Brian', 'Curtis', 'David'];
var i = 0;
var observable = simpleObservable('Adam');
var name = simpleCompute(function(){
return observable.get();
});
var observation = new Observation(null, null, function(newVal, oldVal, batchNum){
var expectedName = expected[i++];
QUnit.equal(name(), expectedName, 'Name has become '+expectedName+' and callback called');
start();
});

QUnit.equal(name(), expected[i++], 'Name starts out Adam');

observation.pushToStack();
name();
observation.popFromStack();

observable.set('Brian');
observable.set('Curtis');
observable.set('David');
});