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

an attempt at adding retry() to trackedTask #119

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions reactiveweb/src/ember-concurrency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,10 @@ export function task<
>(context: object, task: LocalTask, thunk?: () => Args) {
assert(`Task does not have a perform method. Is it actually a task?`, 'perform' in task);

const state = new State<Args, Return, LocalTask>(task);
const state = new State<Args, Return, LocalTask>(task, thunk);

let destroyable = resource(context, () => {
let args = thunk || DEFAULT_THUNK;

let positional = normalizeThunk(args).positional as Args;

state[RUN](positional || []);
state[RUN]();

return state;
});
Expand All @@ -69,7 +65,7 @@ export function task<

registerDestructor(state, () => state[TASK].cancelAll());

return destroyable as unknown as TaskInstance<Return>;
return destroyable as unknown as TaskInstance<Return> & { retry: () => void };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know in discord you said you might want to do the type differently -- but I think this is fine <3

}

export const trackedTask = task;
Expand Down Expand Up @@ -110,16 +106,19 @@ export interface TaskInstance<Return = unknown> extends Promise<Return> {
export const TASK = Symbol('TASK');

const RUN = Symbol('RUN');
const THUNK = Symbol('THUNK');

/**
* @private
*/
export class State<Args extends any[], Return, LocalTask extends TaskIsh<Args, Return>> {
// Set via useTask
declare [TASK]: LocalTask;
declare [THUNK]?: () => Args;

constructor(task: LocalTask) {
constructor(task: LocalTask, thunk?: () => Args) {
this[TASK] = task;
this[THUNK] = thunk;

// eslint-disable-next-line @typescript-eslint/no-this-alias
const self = this;
Expand Down Expand Up @@ -161,6 +160,10 @@ export class State<Args extends any[], Return, LocalTask extends TaskIsh<Args, R
return taskRunner.value;
}

if (key === 'retry') {
return taskRunner.retry;
}

// We can be thennable, but we'll want to entangle with tracked data
if (key === 'then') {
get(taskRunner.currentTask, 'isRunning');
Expand Down Expand Up @@ -193,11 +196,22 @@ export class State<Args extends any[], Return, LocalTask extends TaskIsh<Args, R
return this.lastTask?.value;
}

[RUN] = (positional: Args) => {
[RUN] = () => {
let args = this[THUNK] || DEFAULT_THUNK;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of doing this[THUNK] || DEFAULT_THUNK here, could we move it to the constructor.

this way, we can access the args directly in here (still invoking normalizeThunk as you have it though).


let positional = (normalizeThunk(args).positional ?? []) as Args;

if (this.currentTask) {
this.lastTask = this.currentTask;
}

this.currentTask = this[TASK].perform(...positional);
};

/**
* Will re-invoke the task passed to `trackedTask`
*/
retry = () => {
this[RUN]();
};
}
66 changes: 66 additions & 0 deletions tests/test-app/tests/utils/ember-concurrency/js-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,72 @@ module('useTask', function () {
assert.true(foo.search.isFinished);
assert.false(foo.search.isRunning);
});

test('it runs again when calling retry()', async function (assert) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a testing technique that might help here (and/or in a rendering test)
is using assert.step('name of thing') and then at various intervals calling assert.verifySteps([expected steps]) -- this way you don't necessarily need to be concerned with the exact values if you don't need to be

class Test {
@tracked input = '';

counter = 0;

_search = restartableTask(async (input: string) => {
// or some bigger timeout for an actual search task to debounce
await timeout(0);

this.counter++;

// or some api data if actual search task
return { results: [input, this.counter] };
});

search = trackedTask(this, this._search, () => [this.input]);
}

let foo = new Test();

// task is initiated upon first access
foo.search;
await settled();


assert.strictEqual(foo.search.value, undefined);
assert.false(foo.search.isFinished);
assert.true(foo.search.isRunning);

await settled();

assert.true(foo.search.isFinished);
assert.false(foo.search.isRunning);
assert.deepEqual(foo.search.value, { results: ['', 1] });
assert.strictEqual(foo._search.performCount, 1, 'Task was performed once');

foo.input = 'Hello there!';
await settled();

assert.deepEqual(foo.search.value, { results: ['', 1] }, 'previous value is retained');
assert.false(foo.search.isFinished);
assert.true(foo.search.isRunning);

await settled();

assert.true(foo.search.isFinished);
assert.false(foo.search.isRunning);
assert.deepEqual(foo.search.value, { results: ['Hello there!', 2] });
assert.strictEqual(foo._search.performCount, 2, 'Task was performed twice');


foo.search.retry();

assert.deepEqual(foo.search.value, { results: ['Hello there!', 2] }, 'previous value is retained');
assert.false(foo.search.isFinished);
assert.true(foo.search.isRunning);

await settled();
assert.true(foo.search.isFinished);
assert.false(foo.search.isRunning);
assert.deepEqual(foo.search.value, { results: ['Hello there!', 3] });

assert.strictEqual(foo._search.performCount, 3, 'Task was performed three times');
});
});
});
});
Loading