CompoundObserver does not report changes to ObjectObservers

[speigg]

if I create a CompoundObserver and add a new ObjectObserver to the compound observer, changes to the observed object do not get reported (when calling deliver() on the compound observer)

[rafaelw]

Sorry for the (rather long) delay. I'm curious if you can say more about your specific use case.

[ghost]

Same thing here, as the following example wont deliver anything

var someObject = {};

var observer = new CompoundObserver();
observer.addObserver(new ObjectObserver(someObject));
observer.open(function() {
  return console.log(arguments);
});

someObject.foo = 'bar'

observer.deliver();

[medmunds]

FWIW, this still seems to be a problem. Use case: trying to construct a CompoundObserver over all the objects in an array.

It might be worth noting in the CompoundObserver docs.

var array = [{a: 1, b: 2}, {a: 3, b: 4}];
var observer = new CompoundObserver();
array.forEach(function(item) {
  observer.addObserver(new ObjectObserver(item));
});
observer.open(function() {
  console.log(arguments);
});

array[1].a = 5; // doesn't trigger the CompoundObserver, but you'd expect it to

[jmesserly]

fyi, code is here if you want to try debugging:
https://github.com/Polymer/observe-js/blob/master/src/observe.js#L1094

I'd start at the check method:
https://github.com/Polymer/observe-js/blob/master/src/observe.js#L1124

@medmunds what browser and/or JS runtime are you using? It may need a call to Platform.performMicrotaskCheckpoint: https://github.com/Polymer/observe-js#about-delivery-of-changes
(apologies if you knew about that, it's a common issue folks hit)

[medmunds]

@jmesserly Thanks for the reminder, but I don't think that's it. Electron 0.25.2 (Chrome42 via libchromium, I think). It has native Object.observe and Array.observe, so shouldn't need performMicrotaskCheckpoint if I'm understanding correctly. Also, ObjectObserver notifies successfully when used by itself; it's only missing notifications inside a CompoundObserver.

I'll try debugging a little more and let you know if I see anything suspicious.

[medmunds]

OK, I think I understand why there's a problem. Not sure how to fix.

Here's a fiddle demonstrating the issue.

When CompoundObserver contains an ObjectObserver (or ArrayObserver), the compound's internal value_ stash ends up with a direct reference to the object being observed (not a copy). So a later change to the object goes undetected in CompoundObserver.check_: it's comparing the changed object to itself.

• CompoundObserver.open (via connect_) calls an initial check_,
  which stores the return from ObjectObservable.open into the compound's value_ stash ...
• ... and because ObjectObserver.open returns its own value_ (which was initialized to the object itself), the original object reference is stored in the CompoundObserver instance's value_ array.
• So after you update the object, CompoundObserver.check_ ends up comparing the updated object to itself (the reference stored in the value_ stash) -- which will never find a difference to notify.

Here's a quick way to see that CompoundObserver's value_ ends up with the original object:

var obj = { a: 1 };
var compoundObserver = new CompoundObserver();
compoundObserver.addObserver(new ObjectObserver(obj));
compoundObserver.open(function() {});
compoundObserver.value_[0] === obj
> true // uh-oh... CompoundObserver has cached the actual object being observed

Should CompoundObserver always make a copy of the value before stashing it?

[jmesserly]

Ah, nice investigation! That jogs my memory ... I think CompoundObserver.addObserver was only ever used with PathObservers as an argument. Conceptually it was intended for cases like computing a function, where you observe a bunch of values, then recompute whenever one of them changes.

I'm not sure what it should do semantically in case of ObjectObserver, since the value hasn't changed?

Could your use case use PathObserver instead?

If not, it almost sounds like you need a different, simpler primitive to build on. Is the goal to simply fire a callback when any one ObjectObserver changes? This example code should do that:

var array = [{a: 1, b: 2}, {a: 3, b: 4}];
function callback() {
  console.log(arguments);
}
array.forEach(function(item) {
  new ObjectObserver(item).open(callback);
});

I imagine it wouldn't be too hard to wrap that into something that implements the Observer interface.

It's funny because on one hand, CompoundObserver is pretty close. On the other hand, newObservedSet (used for optimization purposes) is also pretty close, though I think neither is quite what you're looking for.

Maybe a super simple way to make this work would be:

function objectObserverAdaptor(item) {
  var counter = { value: 0 };
  var observer = new ObjectObserver(item);
  observer.open(function() { counter.value++; });
  // TODO: closing this won't know to close the ObjectObserver
  return new PathObserver(counter, 'value');
}

var array = [{a: 1, b: 2}, {a: 3, b: 4}];
var observer = new CompoundObserver();
array.forEach(function(item) {
  observer.addObserver(objectObserverAdaptor(item));
});
observer.open(function() {
  console.log(arguments);
});
array[1].a = 5;

not sure how to fix that TODO, though.

[medmunds]

Yeah, I ended up just maintaining my own array of ObjectObservers, which works fine for my use case (and lets me close them properly). I had originally looked at CompoundObserver because it seemed like it would encapsulate all that maintenance for me.

So, the easy fix would be to have CompoundObserver.addObserver reject anything other than a PathObserver. 😄