From 1515c4a7669e973084c2ef89086e5fc38c07cbcf Mon Sep 17 00:00:00 2001 From: Rafael Weinstein Date: Thu, 12 Dec 2013 11:25:41 -0800 Subject: [PATCH] PathObserver.defineProperty(target, name, model, path) -> Observer.defineProperty(target, name, observable) R=arv BUG= Review URL: https://codereview.appspot.com/41370044 --- src/observe.js | 57 ++++++++++++++++++++++++++------------------------ tests/test.js | 51 +++++++++++++++++++++----------------------- 2 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/observe.js b/src/observe.js index 6d0b5d5..82d2eed 100644 --- a/src/observe.js +++ b/src/observe.js @@ -385,6 +385,10 @@ return this.value_; }, + getValue: function() { + return this.value_; + }, + internalCallback_: function(records) { if (this.state_ != OPENED) return; @@ -435,11 +439,12 @@ this.callback_.apply(this.target_, args); } catch (ex) { Observer._errorThrownDuringCallback = true; - console.error('Exception caught during observer callback: ' + (ex.stack || ex)); + console.error('Exception caught during observer callback: ' + + (ex.stack || ex)); } }, - reset: function() { + discardChanges: function() { if (this.state_ != OPENED) throw Error('Observer is not open'); @@ -696,6 +701,10 @@ PathObserver.prototype = createObject({ __proto__: Observer.prototype, + getValue: function() { + return this.path_.getValueFrom(this.object_); + }, + connect_: function() { if (hasObserve) this.observedSet_ = new ObservedSet(this.boundInternalCallback_); @@ -791,7 +800,8 @@ for (var i = 0; i < this.observed_.length; i = i + 2) { var pathOrObserver = this.observed_[i+1]; var object = this.observed_[i]; - var value = object === observerSentinel ? pathOrObserver.reset() : + var value = object === observerSentinel ? + pathOrObserver.discardChanges() : pathOrObserver.getValueFrom(object, this.observedSet_) if (sync) { @@ -874,9 +884,12 @@ this.callback_.call(this.target_, this.value_, oldValue); }, - reset: function() { - this.value_ = this.getValueFn_(this.observable_.reset()); - return this.value_; + getValue: function() { + return this.observable_.getValue(); + }, + + discardChanges: function() { + return this.getValueFn_(this.observable_.discardChanges()); }, deliver: function() { @@ -925,40 +938,30 @@ } } - // TODO(rafaelw): It should be possible for the Object.observe case to have - // every PathObserver used by defineProperty share a single Object.observe - // callback, and thus get() can simply call observer.deliver() and any changes - // to any dependent value will be observed. - PathObserver.defineProperty = function(target, name, object, path) { - // TODO(rafaelw): Validate errors - path = getPath(path); + Observer.defineProperty = function(target, name, observable) { var notify = notifyFunction(target, name); - - var observer = new PathObserver(object, path, - function(newValue, oldValue) { - if (notify) - notify(PROP_UPDATE_TYPE, oldValue); - } - ); + observable.open(function(newValue, oldValue) { + if (notify) + notify(PROP_UPDATE_TYPE, oldValue); + }); Object.defineProperty(target, name, { get: function() { - return path.getValueFrom(object); + return observable.getValue(); }, set: function(newValue) { - path.setValueFrom(object, newValue); + observable.setValue(newValue); + return newValue; }, configurable: true }); return { close: function() { - var oldValue = path.getValueFrom(object); - if (notify) - observer.deliver(); - observer.close(); + var lastValue = observable.getValue(); + observable.close(); Object.defineProperty(target, name, { - value: oldValue, + value: lastValue, writable: true, configurable: true }); diff --git a/tests/test.js b/tests/test.js index 23a22ba..3f0335a 100644 --- a/tests/test.js +++ b/tests/test.js @@ -328,12 +328,12 @@ suite('ObserverTransform', function() { obj.foo = 2; - assert.strictEqual(4, observer.reset()); + assert.strictEqual(4, observer.discardChanges()); assertNoChanges(); observer.setValue(2); assert.strictEqual(obj.foo, 1); - assert.strictEqual(2, observer.reset()); + assert.strictEqual(2, observer.discardChanges()); assertNoChanges(); observer.close(); @@ -451,7 +451,7 @@ suite('PathObserver Tests', function() { assertNoChanges(); }); - test('Path reset', function() { + test('Path discardChanges', function() { var arr = {}; arr.foo = 'bar'; @@ -462,7 +462,7 @@ suite('PathObserver Tests', function() { assertPathChanges('baz', 'bar'); arr.foo = 'bat'; - observer.reset(); + observer.discardChanges(); assertNoChanges(); arr.foo = 'bag'; @@ -483,7 +483,7 @@ suite('PathObserver Tests', function() { assertPathChanges('bat', 'bar'); observer.setValue('bot'); - observer.reset(); + observer.discardChanges(); assertNoChanges(); observer.close(); @@ -540,10 +540,10 @@ suite('PathObserver Tests', function() { assert.equal(3, observer.open(callback)); path.setValueFrom(obj, 2); - assert.equal(2, observer.reset()); + assert.equal(2, observer.discardChanges()); path.setValueFrom(obj, 3); - assert.equal(3, observer.reset()); + assert.equal(3, observer.discardChanges()); assertNoChanges(); @@ -821,14 +821,14 @@ suite('PathObserver Tests', function() { var b = {}; var c = {}; - root.a.observer = PathObserver.defineProperty(root.a, 'value', - root, 'value'); + root.a.observer = Observer.defineProperty(root.a, 'value', + new PathObserver(root, 'value')); - root.a.b.observer = PathObserver.defineProperty(root.a.b, 'value', - root.a, 'value'); + root.a.b.observer = Observer.defineProperty(root.a.b, 'value', + new PathObserver(root.a, 'value')); - root.c.observer = PathObserver.defineProperty(root.c, 'value', - root, 'value'); + root.c.observer = Observer.defineProperty(root.c, 'value', + new PathObserver(root, 'value')); root.c.value = 2; assert.strictEqual(2, root.a.b.value); @@ -852,8 +852,9 @@ suite('PathObserver Tests', function() { Object.observe(target, callback); } - var observer = PathObserver.defineProperty(target, 'computed', - source, 'foo.bar'); + var observer = Observer.defineProperty(target, 'computed', + new PathObserver(source, 'foo.bar')); + assert.isTrue(target.hasOwnProperty('computed')); assert.strictEqual(1, target.computed); @@ -892,12 +893,6 @@ suite('PathObserver Tests', function() { name: 'computed', type: Observer.changeRecordTypes.add }, - { - object: target, - name: 'computed', - type: Observer.changeRecordTypes.update, - oldValue: 1 - }, { object: target, name: 'computed', @@ -910,12 +905,14 @@ suite('PathObserver Tests', function() { test('DefineProperty - empty path', function() { var target = {} - var observer = PathObserver.defineProperty(target, 'foo', 1, ''); + var observer = Observer.defineProperty(target, 'foo', + new PathObserver(1)); assert.isTrue(target.hasOwnProperty('foo')); assert.strictEqual(1, target.foo); var obj = {}; - var observer2 = PathObserver.defineProperty(target, 'bar', obj, ''); + var observer2 = Observer.defineProperty(target, 'bar', + new PathObserver(obj)); assert.isTrue(target.hasOwnProperty('bar')); assert.strictEqual(obj, target.bar); }); @@ -1175,7 +1172,7 @@ suite('ArrayObserver Tests', function() { assertNoChanges(); }); - test('Array reset', function() { + test('Array discardChanges', function() { var arr = []; arr.push(1); @@ -1190,7 +1187,7 @@ suite('ArrayObserver Tests', function() { }]); arr.push(3); - observer.reset(); + observer.discardChanges(); assertNoChanges(); arr.pop(); @@ -1784,7 +1781,7 @@ suite('ObjectObserver Tests', function() { assertNoChanges(); }); - test('Object reset', function() { + test('Object discardChanges', function() { var obj = {}; obj.foo = 'bar'; @@ -1804,7 +1801,7 @@ suite('ObjectObserver Tests', function() { }); obj.blaz = 'bat'; - observer.reset(); + observer.discardChanges(); assertNoChanges(); obj.bat = 'bag';