Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Commit

Permalink
ensure that observedSet resets observed objects after changes
Browse files Browse the repository at this point in the history
this was a pretty major oversight on my part, and unfortunately, the tests are structure such that the "organic" callback of Object.observe is never depended upon.

What really needs to be done here is to make most of the tests be async using the then() pattern, but this bug is blocking polymer release, so I'm going to do that work in a follow-on patch.

#42
R=arv
BUG=

Review URL: https://codereview.appspot.com/53490043
  • Loading branch information
rafaelw committed Jan 16, 2014
1 parent 5b237da commit 0152d54
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
2 changes: 2 additions & 0 deletions src/observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@

observer.check_();
}

scheduleReset();
}

var record = {
Expand Down
43 changes: 39 additions & 4 deletions tests/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ var callbackInvoked = false;

window.testingExposeCycleCount = true;

function then(fn) {
setTimeout(function() {
Platform.performMicrotaskCheckpoint();
fn();
}, 0);

return {
then: function(next) {
return then(next);
}
};
}

function noop() {}

function callback() {
Expand All @@ -38,8 +51,9 @@ function assertNoChanges() {
assert.isUndefined(callbackArgs);
}

function assertPathChanges(expectNewValue, expectOldValue) {
observer.deliver();
function assertPathChanges(expectNewValue, expectOldValue, dontDeliver) {
if (!dontDeliver)
observer.deliver();

assert.isTrue(callbackInvoked);

Expand All @@ -48,8 +62,10 @@ function assertPathChanges(expectNewValue, expectOldValue) {
assert.deepEqual(expectNewValue, newValue);
assert.deepEqual(expectOldValue, oldValue);

assert.isTrue(window.dirtyCheckCycleCount === undefined ||
window.dirtyCheckCycleCount === 1);
if (!dontDeliver) {
assert.isTrue(window.dirtyCheckCycleCount === undefined ||
window.dirtyCheckCycleCount === 1);
}

callbackArgs = undefined;
callbackInvoked = false;
Expand Down Expand Up @@ -604,6 +620,25 @@ suite('PathObserver Tests', function() {
observer.close();
});

test('Path - root is initially null', function(done) {
var model = { };

var path = Path.get('foo');
observer = new PathObserver(model, 'foo.bar');
observer.open(callback);

model.foo = { };
then(function() {
model.foo.bar = 1;

}).then(function() {
assertPathChanges(1, undefined, true);

observer.close();
done();
});
});

test('Path With Indices', function() {
var model = [];

Expand Down

0 comments on commit 0152d54

Please sign in to comment.