tracked-tools / tracked-built-ins

Tracked versions of JavaScript's built-in classes
MIT License
128 stars 24 forks source link

Decide on design for (and implement?) `TrackedObject` constructor #318

Open chriskrycho opened 2 years ago

chriskrycho commented 2 years ago

Note: we can't actually start on this yet because Framework still needs to figure out what the semantics should be for it!

The simplest idea, “match the built-in constructors exactly” can have some weird effects:

let original = { foo: true };
let viaObject = new Object(original);
let viaTrackedObject = new TrackedObject(original);

original.foo = false;
console.log(viaObject.foo); // `false`
console.log(viaTrackedObject); // ???

viaTrackedObject.foo = true;
console.log(original.foo); // ???
console.log(viaObject.foo); // ???

Additionally, we cannot ever make new TrackedObject() return the desired type from a TypeScript point-of-view.


Related: #73.

void-mAlex commented 1 year ago

let me know if you see this as a standalone issue but I've found the current approach of cloning the object to use as a target of the Proxy breaks for private class properties

class MyObject {
  #original = null;
  //other props that I want to track
  get hasChanges(){
    return this.#original !== null && this.#original; //your changes checks here
  }
}
// elsewhere in code
const myObject = new MyObject();
const trackedObject  = new TrackedObject(myObject);
console.log(trackedObject.hasChanges); // error as private properties cannot be accessed

wondering if TrackedObject should instead be extended when trying to use it with private fields or if that is something we want to avoid altogether.

ef4 commented 1 year ago

wondering if TrackedObject should instead be extended when trying to use it with private fields

I checked and this works. And I think class MyThing extends TrackedObject {} is more coherent than new TrackedObject(new MyThing()), but I don't really love either.

I would be happier with a TrackedObject constructor that was limited to one-time field copying at construction, with no attempt at sharing a prototype or descriptors. Both because I don't think we can ever make private fields work, and because I'm dubious that this pattern is ever better than the alternatives.

If you don't know the shape of your data, it's incorrect to be mingling it with any prototype anyway. That's a prototype pollution bug waiting to happen. With unknown shapes you want the equivalent of Object.create(null).

If you do know the shape of your data, you can use @tracked normally.

If you don't control the implementation of the object and it has its own notion of reactivity that differs from @glimmer/tracking: that is the problem starbeam is working on and you aren't going to correctly solve it with just TrackedObject anyway.