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

Commit 6da3137

Browse files
committed
make PathObserver.defineProperty take four arguments and avoid needless allocation
R=arv BUG= Review URL: https://codereview.appspot.com/29320043
1 parent 809a851 commit 6da3137

File tree

2 files changed

+19
-34
lines changed

2 files changed

+19
-34
lines changed

src/observe.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -887,36 +887,35 @@
887887
// every PathObserver used by defineProperty share a single Object.observe
888888
// callback, and thus get() can simply call observer.deliver() and any changes
889889
// to any dependent value will be observed.
890-
PathObserver.defineProperty = function(object, name, descriptor) {
890+
PathObserver.defineProperty = function(target, name, object, path) {
891891
// TODO(rafaelw): Validate errors
892-
var obj = descriptor.object;
893-
var path = getPath(descriptor.path);
894-
var notify = notifyFunction(object, name);
892+
path = getPath(path);
893+
var notify = notifyFunction(target, name);
895894

896-
var observer = new PathObserver(obj, descriptor.path,
895+
var observer = new PathObserver(object, path,
897896
function(newValue, oldValue) {
898897
if (notify)
899898
notify(PROP_UPDATE_TYPE, oldValue);
900899
}
901900
);
902901

903-
Object.defineProperty(object, name, {
902+
Object.defineProperty(target, name, {
904903
get: function() {
905-
return path.getValueFrom(obj);
904+
return path.getValueFrom(object);
906905
},
907906
set: function(newValue) {
908-
path.setValueFrom(obj, newValue);
907+
path.setValueFrom(object, newValue);
909908
},
910909
configurable: true
911910
});
912911

913912
return {
914913
close: function() {
915-
var oldValue = path.getValueFrom(obj);
914+
var oldValue = path.getValueFrom(object);
916915
if (notify)
917916
observer.deliver();
918917
observer.close();
919-
Object.defineProperty(object, name, {
918+
Object.defineProperty(target, name, {
920919
value: oldValue,
921920
writable: true,
922921
configurable: true

tests/test.js

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -722,20 +722,14 @@ suite('PathObserver Tests', function() {
722722
var b = {};
723723
var c = {};
724724

725-
root.a.observer = PathObserver.defineProperty(root.a, 'value', {
726-
object: root,
727-
path: 'value'
728-
});
725+
root.a.observer = PathObserver.defineProperty(root.a, 'value',
726+
root, 'value');
729727

730-
root.a.b.observer = PathObserver.defineProperty(root.a.b, 'value', {
731-
object: root.a,
732-
path: 'value'
733-
});
728+
root.a.b.observer = PathObserver.defineProperty(root.a.b, 'value',
729+
root.a, 'value');
734730

735-
root.c.observer = PathObserver.defineProperty(root.c, 'value', {
736-
object: root,
737-
path: 'value'
738-
});
731+
root.c.observer = PathObserver.defineProperty(root.c, 'value',
732+
root, 'value');
739733

740734
root.c.value = 2;
741735
assert.strictEqual(2, root.a.b.value);
@@ -759,10 +753,8 @@ suite('PathObserver Tests', function() {
759753
Object.observe(target, callback);
760754
}
761755

762-
var observer = PathObserver.defineProperty(target, 'computed', {
763-
object: source,
764-
path: 'foo.bar'
765-
});
756+
var observer = PathObserver.defineProperty(target, 'computed',
757+
source, 'foo.bar');
766758
assert.isTrue(target.hasOwnProperty('computed'));
767759
assert.strictEqual(1, target.computed);
768760

@@ -819,18 +811,12 @@ suite('PathObserver Tests', function() {
819811

820812
test('DefineProperty - empty path', function() {
821813
var target = {}
822-
var observer = PathObserver.defineProperty(target, 'foo', {
823-
object: 1,
824-
path: ''
825-
});
814+
var observer = PathObserver.defineProperty(target, 'foo', 1, '');
826815
assert.isTrue(target.hasOwnProperty('foo'));
827816
assert.strictEqual(1, target.foo);
828817

829818
var obj = {};
830-
var observer2 = PathObserver.defineProperty(target, 'bar', {
831-
object: obj,
832-
path: ''
833-
});
819+
var observer2 = PathObserver.defineProperty(target, 'bar', obj, '');
834820
assert.isTrue(target.hasOwnProperty('bar'));
835821
assert.strictEqual(obj, target.bar);
836822
});

0 commit comments

Comments
 (0)