machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
690 stars 157 forks source link

Issue with `isFinished` after updating to 3.15 #340

Closed mydea closed 4 years ago

mydea commented 4 years ago

Possibly related to #337, I get

Assertion Failed: You attempted to update `isFinished` on `<Task:_resolvePromise.perform()>`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

After trying to update to 3.15 & latest ember-concurrency.

The code triggering this is a bit special, but it worked fine before:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { restartableTask } from 'ember-concurrency-decorators';
import { timeout } from 'ember-concurrency';

/*
 * This component takes a `promise` argument and yields an object with a `value` and a `isPending` property.
 * Note that the `value` will only change once the passed in promise is resolved!
 * This means that the last value will be used until a new value is resolved - it will not f.e. switch to `null` in between or similar.
 */
export default class AwaitPromise extends Component {
  /*
   * Arguments:
   *  - promise
   */

  @tracked _resolvedValue = undefined;
  @tracked _isRunning = false;
  @tracked _hasNeverResolved = true;
  @tracked _promise = undefined;

  get promiseData() {
    let { resolvedValue, isPending } = this;

    return {
      value: resolvedValue,
      isPending
    };
  }

  get isPending() {
    if (!this.args.promise) {
      return false;
    }

    return this._hasNeverResolved || this._isRunning;
  }

  get resolvedValue() {
    let { promise } = this.args;

    if (!promise) {
      this._resolvedValue = undefined;
      this._promise = undefined;
      return undefined;
    }

    let { _resolvedValue } = this;

    if (_resolvedValue && promise === this._promise) {
      return _resolvedValue;
    }

    if (promise !== this._promise) {
      this._resolvePromise.perform(promise);
    }

    return _resolvedValue;
  }

  @restartableTask
  *_resolvePromise(promise) {
    this._isRunning = true;

    // We have to wait a tick before setting _promise,
    // otherwise it complains that we set a property after accessing it in a getter
    yield timeout(1);

    this._promise = promise;

    this._resolvedValue = yield promise;

    yield timeout(1);

    this._hasNeverResolved = false;
    this._isRunning = false;
  }
}

Usage:

<AwaitPromise @promise={{this.promise}} as |promise|>
  {{#if promise.isLoading}}
    LOADING
  {{else}}
    {{log promise.value}}
  {{/if}}
</AwaitPromise>
maxfierke commented 4 years ago

Yep, thanks for the report. Definitely similar to #337. Looks like we'll need to more aggressively remove uses of get from @ember/object internally.