Deprecate `recompute` on `Helper`?

[chancancode]

The point of the recompute method is that you call it to trigger Ember to re-render the helper in some kind of callback, such as observers, promise callbacks, etc.

Since helpers are now auto-tracked, it should be possible to refactor any usage of recompute into setting tracked state and accomplish the same thing. In most cases, the transformation is pretty straightforward.

Before:

class MyHelper extends Helper {
  @service foo;

  @observes('foo.bar.[]') onBarChanged() {
    this.recompute();
  }

  compute() {
    return this.foo.bar.map(...);
  }
}

After:

class MyHelper extends Helper {
  @service foo;

  @tracked bar = this.foo.bar;

  @observes('foo.bar.[]') onBarChanged() {
    this.bar = this.foo.bar;
  }

  compute() {
    return this.bar.map(...);
  }
}

The example uses an observer, because that is a common kind of callback used in this position, but it's ultimately unrelated to my point.I also think there are better ways to accomplish the same thing without using observers, in general, but that is also besides the point here.

For prosperity, here is a second example that doesn't use observers:

Before:

const UNINITIALIZED = Object.freeze({});

class AwaitHelper extends Helper {
  resolved = undefined;
  promise = UNINITIALIZED;

  compute([promise]) {
    if (promise !== this.promise) {
      this.resolved = undefined;
      this.promise = promise;

      Promise.resolve(promise).then(resolved => {
        if (promise === this.promise) {
          this.resolved = resolved;
          this.recompute();
        }
      });
    }

    return this.resolved;
  }

  willDestroy() {
    this.promise = UNINITIALIZED;
  }
}

After:

const UNINITIALIZED = Object.freeze({});

class AwaitHelper extends Helper {
  @tracked resolved = undefined;
  promise = UNINITIALIZED;

  compute([promise]) {
    if (promise !== this.promise) {
      this.resolved = undefined;
      this.promise = promise;

      Promise.resolve(promise).then(resolved => {
        if (promise === this.promise) {
          this.resolved = resolved;
        }
      });
    }

    return this.resolved;
  }

  willDestroy() {
    this.promise = UNINITIALIZED;
  }
}

The general pattern is: instead of calling recompute(), just refactor into setting a piece of tracked state and use that tracked state in the compute() method.

Would be interested to see if there are any valid use cases this doesn't cover.

[rwjblue]

I think it is a tad difficult to suggest "just use @tracked" at the moment, since we also expect addons to continue supporting [email protected] which doesn't support tracking concepts. It is also non-trivial to write code that works for both styles (without completely duplicating the helper).

Either way, I agree with this direction. My point above is that we should probably tie this deprecation to land no earlier than 3.21, at which point 3.12 will no longer be an active concern (since 3.16 and 3.20 would be the active LTS versions). Note: 3.21 is current ember-source master, so this is still a great time to push this forward. :smile:

[chancancode]

3.21 is current ember-source master

What a time to be alive 😄

[mixonic]

We discussed this again today at the framework meeting. As the tracked system is now very, very common, this proposal is definitely ready for RFC and advancement.

[wagenet]

@mixonic who will write the RFC?

[wagenet]

Should this be considered with #832 or separately?