trackedTask returns last task value if current task value is null or undefined

[sergey-zhidkov]

In case if task was called several times and the last returned value was null or undefined, then trackedTask returns last (previous) value instead of the correct current one.

For example:

import Component from '@glimmer/component';
import { keepLatestTask } from 'ember-concurrency';
import { trackedTask } from 'ember-resources/util/ember-concurrency';
import { taskFor } from 'ember-concurrency-ts';

export default class ChildComponent extends Component {
  resultTask = trackedTask(this, taskFor(this.myTask), () => [
    this.args.myArg,
  ]);

  get value() {
    return this.resultTask.value;
  }

  getResultFromArg(myArg) {
    if (myArg === 0) {
      return 'non null non undefined result';
    } else {
      // This value will be lost if it was returned as the latest result of task run
      return undefined;

      // This value will be lost as well
      // return null;

      // This one will be returned from "trackedTask" as expected
      // return 'result';
    }
  }

  @keepLatestTask
  *myTask(myArg) {
    const result = yield this.getResultFromArg(myArg);
    console.log('result in context of ', myArg, '>>', result);
    return result;
  }
}

Template

<div>
  <div>value</div>
  <div>
    <strong>{{this.value}}</strong>
  </div>
</div>

Steps to reproduce:
step 1: we pass "0" as value of "myArg" to Child Component => myTask will return "non null non undefined result" value. It will set this.currentTask value to non null non undefined result at https://github.com/NullVoxPopuli/ember-resources/blob/main/ember-resources/src/util/ember-concurrency.ts#L225.
step 2: we pass "1" as value of "myArg" to Child Component => myTask will return undefined value. It will set this.currentTask value to undefined.
step 3: test that getter returns non null non undefined result instead of expected undefined.

get value() {
    return this.resultTask.value; // will return `non null non undefined result`
}

I believe this is happening because of the code in TaskResource class https://github.com/NullVoxPopuli/ember-resources/blob/main/ember-resources/src/util/ember-concurrency.ts#L217

  get value() {
    return this.currentTask.value ?? this.lastTask?.value;
  }

I wonder if this is expected behavior.

[NullVoxPopuli]

Is this with v5 or the v6beta?

[NullVoxPopuli]

I wonder if this is expected behavior

It is not, and i think you investigated the bug beautifully!

The value getter probably needs to consider if current task is finished before deciding to return lastTask.value or currentTask.value, rather than the existing naiive ??

Excellent find!

[NullVoxPopuli]

@sergey-zhidkov would you be willing to submit a PR?

maybe something to the effect of:

  get value() {
    if (this.return.currentTask?.isFinished) return this.currentTask?.value;
    
    return this.lastTask?.value;
  }

?

[sergey-zhidkov]

It's v5.

Excellent find!

Thank you.

The value getter probably needs to consider if current task is finished before deciding to return lastTask.value or currentTask.value, rather than the existing naiive ??

I do agree. This is a good way to fix the issue.

@sergey-zhidkov would you be willing to submit a PR?

I'll be happy to. I'll try to do it by the end of Wednesday. Probably will take some time to write proper tests.

[sergey-zhidkov]

Fix was merged to main.