emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.45k stars 4.21k forks source link

Improve compatibility of tracked properties and native proxies #18781

Open mwpastore opened 4 years ago

mwpastore commented 4 years ago

Context

The built-in Reflect object gives developers the ability to mask or override the this of an object's property accessors by specifying an alternative thisArg at call time. This is useful for applying the behavior of an existing object in a modified context, without cloning or altering the underlying object. (And probably for other use-cases as well, but this is the one I'm interested in.)

Consider the following example:

class Foo {
  constructor(num) {
    this.num = +num;
  }

  get doubleNum() {
    return this.num * 2;
  }
}

I'm going to create a new instance of this class and send it off somewhere to be used. I'd like to make all of the advanced wizardry of Foo#doubleNum available downstream, but I don't want any pesky consumers directly modifying my object. (Yes, like a change set. I'm talking about change sets.) What I can do is pass down a native Proxy object instead of the "raw" object.

Something like:

function makeProxy(obj) {
  const buffer = new Map();

  return new Proxy(obj, {
    get(target, key) {
      if (key === 'num' && buffer.has(key)) {
        return buffer.get(key);
      }

      return target[key];
    },

    set(target, key, value) {
      if (key === 'num') {
        buffer.set(key, value);

        return true;
      }

      /*
       * If we were to make the other properties of our underlying
       * object writable...
       *
      target[key] = value;

      return true;
       */

      console.error('oopsie! this object is (mostly) read-only');

      return false;
    }
  });
}

So far so good. Downstream consumers can read and write the num property without modifying myFoo, and everything else is read-only. And with a few more lines of code, I can easily add the ability to apply the buffered changes at a time of one's choosing (e.g. after a user clicks the "Save" button).

But there's one problem:

const
  myFoo = new Foo(10),
  myFooProxy = makeProxy(myFoo);

myFooProxy.num = 500; // => 500
myFooProxy.doubleNum; // => 20 (???)

myFoo.num; // => 10
myFoo.doubleNum; // => 20

myFooProxy.doubleNum is always 20—regardless of the value of myFooProxy.num—because the this of the getter method is the underlying myFoo instance of Foo—as you'd expect—and myFoo has no access to the change buffer. What we need to do is override the this of #doubleNum at call time so it can retrieve the buffered value of num from the Proxy object.

This is where Reflect comes in:

function makeProxy(obj) {
  const buffer = new Map();

  return new Proxy(obj, {
    get(target, key, receiver) { // <-- HERE
      if (key === 'num' && buffer.has(key)) {
        return buffer.get(key);
      }

      return Reflect.get(target, key, receiver); // <-- HERE
    },

    set(target, key, value, receiver) { // <-- HERE
      if (key === 'num') {
        buffer.set(key, value);

        return true;
      }

      /*
       * If we were to make the other properties of our underlying
       * object writable...
       *
      Reflect.set(target, key, value, receiver); // <-- HERE

      return true;
       */

      console.error('oopsie! this object is (mostly) read-only');

      return false;
    }
  });
}

We take the additional receiver argument of the Proxy handler's get and set traps and pass it along via Reflect.get and Reflect.set. This overrides the this of our property accessors and lets myFooProxy.doubleNum get the buffered value of num from the Proxy object.

Now everything works how we want:

const
  myFoo = new Foo(10),
  myFooProxy = makeProxy(myFoo);

myFooProxy.num = 500; // => 500
myFooProxy.doubleNum; // => 1000 (!!!)

myFoo.num; // => 10
myFoo.doubleNum; // => 20

Problems

So why am I submitting this long, rambling issue report about (checks notes) JavaScript built-ins to the Ember.js repo? Simply put, the implementation of tracked properties in Ember is incompatible with this pattern. As soon as you add @tracked num; to the top of Foo's class definition, you get some spooky behavior.

I've identified the following "areas of concern":

  1. The value of a tracked property—when assigned via an underlying object's constructor or some other unmasked method—is inaccessible via reflection. The this of e.g. a constructor is always the object being constructed, so the given value will be stored in the underlying object's slot of the tracked property's values WeakMap. When you try to retrieve it with Reflect.get(underlyingObject, key, proxyObject), it looks in the Proxy object's slot instead.
  2. The value of a tracked property—when assigned via reflection—does not "punch through" to the underlying object. This is the reverse of the above issue, i.e. when a tracked property is mutated with Reflect.set(underlyingObject, key, value, proxyObject). The Proxy object is implicitly buffering tracked properties... whether you're aware of it or not! N.B. To be fair, this is a weird one, because you probably wouldn't want to mask the this of most setter methods. But if a developer's mental model of tracked properties is more "data descriptors" than "accessor descriptors", this turns into one heck of a pitfall.
  3. Buffered tracked properties are not tracked. This one is "obvious", but worth mentioning as a footgun and to draw contrast with the next point. The solution is to use a tracking-aware buffer object, e.g. a TrackedMap or ember-changeset.
  4. Autotracking can't "pierce the veil" for un-buffered (passed-through) tracked properties. If an underlying tracked property is mutated by some other means, the Proxy handler's get trap does not fire. Open Question: Perhaps because the tracked property accessors pass the "wrong" this to tagForProperty()?

Workarounds

The solution I've employed thus far is to check for tracked properties in my Proxy handlers and omit the receiver argument to Reflect.get and Reflect.set (or simply use Ember.get and Ember.set instead). Unfortunately, I don't know of any particularly good way to determine at runtime whether or not a property is tracked, so I'm clumsily searching up the prototype chain to find the descriptor and inspecting descriptor.get.toString().

Solutions

I don't think I understand the problem or the implementation of tracked properties well enough to know what the "right" solution is, but I have a few ideas of what would be helpful for my use-case:

pzuraq commented 4 years ago

FWIW, both WeakMaps and private fields break Proxies like this. It’s a known issue, and TC39 has been discussing it, but there doesn’t seem to be any consensus to do anything about it. Personally, this is why I’m avoiding using Proxies to wrap arbitrary objects/classes.

In terms of the best way to solve this, I think that honestly the best option long term would be to do what we want to do once decorators advance, which is use a private symbol on the instance instead of a WeakMap. This would likely be both more efficient and prevent this issue (I think?)

The reason we didn’t do it now is there isn’t a way to define that symbol during construction, since we convert the property to a getter, and getters cannot be initialized with the current decorators transform. This means that the object shape would be unpredictable, which is a huge cost for perf 😕

mwpastore commented 4 years ago

@pzuraq I figured it was a known issue, and I'm glad to hear that a potential solution is out there, albeit in the long term.

Any thoughts on a not-so-dirty way to determine whether or not a property is tracked?