From acbf03d723f904f7fb3ad6758298eb99a0309227 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 20 Mar 2023 12:11:13 -0400 Subject: [PATCH] fix: #835 incorrect value returned when last task was dropped Co-Authored-By: rpemberton <19560029+rpemberton@users.noreply.github.com> --- .changeset/mean-moons-grab.md | 6 ++++ ember-resources/src/util/ember-concurrency.ts | 2 +- .../tests/utils/ember-concurrency/js-test.ts | 34 ++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 .changeset/mean-moons-grab.md diff --git a/.changeset/mean-moons-grab.md b/.changeset/mean-moons-grab.md new file mode 100644 index 000000000..9e271f7be --- /dev/null +++ b/.changeset/mean-moons-grab.md @@ -0,0 +1,6 @@ +--- +"ember-resources": patch +--- + +Fixes [#835](https://github.com/NullVoxPopuli/ember-resources/issues/835) - resolves regression introduced by [PR: #808 ](https://github.com/NullVoxPopuli/ember-resources/pull/808) which aimed to correctly return the _previous_ task instance's value if the _current task_ hasn't finished yet. The regression described by #835 was that if a task in cancelled (e.g.: dropped), it is considered finished, and that canceled task's value would be used instead of the last compuleted task. In normal ember-concurrency APIs, this is abstracted over via the `.lastSuccessful` property on the `TaskProperty`. The goal of the `.value` on `trackedTask` is to mimic the property chain: `taskProperty.lastSuccessful?.value`. + diff --git a/ember-resources/src/util/ember-concurrency.ts b/ember-resources/src/util/ember-concurrency.ts index f031e819b..da4fbd7ea 100644 --- a/ember-resources/src/util/ember-concurrency.ts +++ b/ember-resources/src/util/ember-concurrency.ts @@ -214,7 +214,7 @@ export class TaskResource< declare lastTask: TaskInstance | undefined; get value() { - if (this.currentTask?.isFinished) { + if (this.currentTask?.isFinished && !this.currentTask.isCanceled) { return this.currentTask.value; } diff --git a/test-app/tests/utils/ember-concurrency/js-test.ts b/test-app/tests/utils/ember-concurrency/js-test.ts index ec764cbd7..96d67a3d4 100644 --- a/test-app/tests/utils/ember-concurrency/js-test.ts +++ b/test-app/tests/utils/ember-concurrency/js-test.ts @@ -1,10 +1,11 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { tracked } from '@glimmer/tracking'; import { settled } from '@ember/test-helpers'; +import { waitFor } from '@ember/test-waiters'; import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; -import { restartableTask, timeout } from 'ember-concurrency'; +import { dropTask, restartableTask, timeout } from 'ember-concurrency'; import { taskFor } from 'ember-concurrency-ts'; import { trackedTask } from 'ember-resources/util/ember-concurrency'; @@ -168,6 +169,37 @@ module('useTask', function () { assert.false(foo.search.isRunning); assert.strictEqual(foo.search.value, null); }); + + test('it returns correct task value when tasks have been dropped', async function (assert) { + class Test { + @tracked input = 'initial value'; + + search = trackedTask(this, taskFor(this._search), () => [this.input]); + + @dropTask + @waitFor + *_search(input: string) { + yield new Promise((resolve) => setTimeout(() => resolve(''))); + + return input; + } + } + + let foo = new Test(); + + // task is initiated upon first access + foo.search.value; + + // immediately start another task (this will be dropped/cancelled) + foo.input = 'updated value'; + foo.search.value; + + await settled(); + + assert.strictEqual(foo.search.value, 'initial value', 'returns value from first task'); + assert.true(foo.search.isFinished); + assert.false(foo.search.isRunning); + }); }); }); });