canjs / can-simple-observable

Create an observable value.
https://canjs.com/doc/can-simple-observable.html
MIT License
3 stars 1 forks source link

Should unbound ResolverObservables return their _value if it exists #45

Closed phillipskevin closed 5 years ago

phillipskevin commented 5 years ago

Going through one of our examples, I was confused by the behavior of a ResolverObservable that calls resolve synchronously like this:

    prop: String,
    count: {
      value({ listenTo, resolve }) {
        let count = resolve(0); // synchronously calling resolve
        listenTo("prop", () => resolve(++count));
      }
    }

For properties like this, if they are bound and then unbound, they no longer retain their value:

const obs = new Obs();

obs.listenTo("count", ({ value }) => {
  console.log(`count changed to ${value}`);  // -> "count changed to 1"
});

obs.prop = "Hi";

obs.stopListening();

console.log(`count: ${obs.count}`); // -> "count: 0"

This happens because when the property is read after it is no longer bound, the value function is called again and the _value is set to the synchronously resolved value:

https://github.com/canjs/can-simple-observable/blob/42e61bcce4455b73715f19e25305071f3c20c64a/resolver/resolver.js#L205-L213

Here is a codepen showing the issue: https://codepen.io/kphillips86/pen/oNvYEMa?editors=0010.

justinbmeyer commented 5 years ago

In the example that you gave, the this._value will not ever update:

obs.prop = "Hi";
obs.count //-> 1

obs.stopListening();

obs.prop = "Bye";
obs.count //-> 1

obs.prop = "Adios";
obs.count //-> 1

IMO, this is also very odd. At least by calling the "deriving" function over again, we have some signal that the value needs to be determined at that time, that it is not static.

I think the "right" solution to this is to enable "weakly" bound properties.

    count: {
      weak: ["prop"],
      value({ listenTo, resolve }) {
        let count = resolve(0); // synchronously calling resolve
        listenTo("prop", () => resolve(++count));
      }
    }

This would bind count as long as prop was bound. Ideally, we could do this by adding another level to key-tree. On branch for "weak" bindings, another for "user" bindings. This is because once all "user" bindings are removed, we'd have to tear down the weak bindings.

I think there's an issue about this somewhere.

phillipskevin commented 5 years ago

46 didn't change this behavior; however, it made it so that you can set the resetUnboundValueInGet flag so that when the get() is called for an unbound property, the value function will be called and the value will be whatever value is resolved synchronously or undefined.

This means the behavior is now consistent whether or not resolve is called synchronously in the value function.