Make class fields observable

[phillipskevin]

We should eventually make this work:

class Obj extends ObservableObject {
  prop = "Hello";
  static props = {};
}

const obj = new Obj();
obj.on("prop", () => {
    console.log("prop changed");
});

obj.prop = "Hi";

[justinbmeyer]

var base = new Proxy({},{
  defineProperty(){
    console.log("hit")
  }
});
var obj = Object.create(base);
Object.defineProperty(obj, "foo", {value: "abc"})

[justinbmeyer]

This is prevented by tc39/proposal-class-fields#151 (comment)

I wrote up a bit on why in that thread: tc39/proposal-class-fields#151 (comment)

[justinbmeyer]

I think we could do this with ObservableObject, but not with StacheElement:

class HelloWorld extends StacheElement {
  static view = `<h1>Hello {{this.subject}}</h1>`;
  subject = "World";
}
customElements.define("hello-world", HelloWorld);

[justinbmeyer]

Solution BrainStorm

Parsing

Could we parse?

class Obj3 {
  foo = "bar"
  constructor(){
  }
}
Obj3.toString() // -> includes `foo = "bar"`

Call a method

We could ask folks to call a method:

class HelloWorld extends StacheElement {
  static view = `<h1>Hello {{this.subject}}</h1>`;
  subject = "World";
  constructor(){
    super();
    this.checkForProps();
  }
}

Check for these props in lifecycles

Like calling a method, we could check for these properties in the lifecycle methods.

This would mean that new MyElement().initialize() will always need to be called as new MyElement({foo: "bar"}) will not be able to make any class fields observable.

Post initialization hook

If there was a way to capture when new MyElement() was complete, we could look for these properties.

Decorators could help here, we could make sure the final constructor has some "completion" code.

Overwrite Object.defineProperty

super() doesn't directly use Object.defineProperty() to set these properties.

Wait for decorators

https://github.com/tc39/proposal-decorators#set

class HelloWorld extends StacheElement {
  static view = `<h1>Hello {{this.subject}}</h1>`;
  @set subject = "World";
}

[phillipskevin]

For ObservableObject/ObservableArray we can proxy instances, so we could make these constructors return a proxy that traps defineProperty and makes these observable.

For StacheElement, one of the solutions above could be used. I think doing this check in initialize() makes sense.

If that works though, maybe we should just do the same in initialize for the observables as well to keep the behavior consistent.

[matthewp]

I think the initialize() suggestion will work fine for StacheElement.

For ObservableObject/ObservableArray, however, the "proxy instance" idea would have a few downsides.

1. There is already a proxy on the prototype. Would we remove that and only have one on the instance or have 2?
2. All of the code ported from can-define operates on defineProperty. If we are trapping defineProperty how do we differentiate between calls made by fields vs. calls made by define.property, etc?
3. This method would not work with static seal = true

[justinbmeyer]

Can you elaborate on the reasons for #3?

[matthewp]

Our static seal = true option seal the instance inside of the constructor. Fields define after the super() calls, so it will throw. So there's an inherit incompatibility between how fields work and how our seal behavior works, if sealing happens in the constructor phase (as opposed to the initialize phase).

[phillipskevin]

For ObservableObject/ObservableArray, could we also do it in initialize()?

[matthewp]

Currently initialize() is called in the constructor. Where would we move it to?

[phillipskevin]

All of the code ported from can-define operates on defineProperty. If we are trapping defineProperty how do we differentiate between calls made by fields vs. calls made by define.property, etc?

We don't ever defineProperty on the instance, right? I don't think this is an issue.

[justinbmeyer]

@matthewp (not saying we should do this), but we could make a "weak" seal ... essentially allow Object.defineProperty() sets, but not foo.bar = value sets. This would probably catch 99% of the user problems that an actual seal would catch.

[phillipskevin]

We just found another issue this would fix. If you set a property on an element instance before it is upgraded (before customElements.define is called), its observability is broken: https://codepen.io/kphillips86/pen/GRKNgVw?editors=1010

It's not incredibly likely that people will do this, but it should be possible.

[phillipskevin]

@cherifGsoul for ObservableArray, if someone does this:

class Foo extends ObservableArray {
  static items = type.convert(ObservableObject);
  items = "Baz";
}

We should warn for the items field and suggest using static items instead.

ObservableArray does not support a class field named items. Try using a different name or using static items