Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.volatile deprecation in 3.9 needs an alternative for "classic" classes #17709

Closed
kategengler opened this issue Mar 6, 2019 · 4 comments · Fixed by #17710
Closed

.volatile deprecation in 3.9 needs an alternative for "classic" classes #17709

kategengler opened this issue Mar 6, 2019 · 4 comments · Fixed by #17710

Comments

@kategengler
Copy link
Member

In 3.9 .volatile is deprecated. The deprecation guide suggests moving to native classes, but that isn't yet possible in all cases, and so we need an alternative to .volatile to allow the deprecation to be resolved where native classes cannot yet be used.

h/t @pzuraq

@rwjblue
Copy link
Member

rwjblue commented Mar 6, 2019

Over in the RFC I mentioned this as well (see emberjs/rfcs#370 (comment)):


Because the only path forward involves using native classes, does the deprecation of .volatile() need to be tied to the deprecation of Ember.Object.extend({}) style syntax?

No 😸, and native classes aren't the only way forward (though the alternatives are less ergonomic than ES classes).

For example, you can migrate from:

export default Component.extend({
  init() {
    this._super(...arguments);
    this._foo = 0;
  },

  foo: computed(function() {
    return this._foo++;
  }).volatile()
});

To:

const FooBar = Component.extend({
  init() {
    this._super(...arguments);
    this._foo = 0;
  }
});

Object.defineProperty(FooBar.prototype, 'foo', {
  get() {
    return ++this._foo;
  }
});

export default FooBar;

@kategengler
Copy link
Member Author

Then it may be that just the deprecation guide needs to change.

@kanongil
Copy link
Contributor

I have 2 use cases that aren't covered:

Class extend inheritance:

const A = Component.extend({
  foo: computed(function() {
    return { foo: true };
  }).volatile()
});

const B = Component.extend({
  foo: computed(function() {
    let obj = this._super();
    obj.bar = 123;
    return obj;
  }).volatile()
});

The second case is for an internal addon, which exposes a ComputedProperty instance that needs to call the _getter on all get() calls. This case fundamentally clashes with the deprecation.

@kanongil
Copy link
Contributor

3rd case that is not covered: Relying on dependent properties.

Example:

  currentLimitMs: computed('program.{start,duration}', function () {
    cancel(this._invalidateNow);

    let start = this.get('program.start');
    let duration = this.get('program.duration');

    let now = Date.now();
    let limitMs = now - start;
    if (limitMs < duration) {
      this._invalidateNow = later(() => this.notifyPropertyChange('currentLimitMs'), 1000);
    } else {
      limitMs = duration;
    }

    return limitMs;
  }).volatile(),

Any chance that the deprecation can be reverted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants