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

Make status of Task a readonly field #4552

Open
wants to merge 3 commits 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/smart-pianos-kick.md
@@ -0,0 +1,10 @@
---
'@lit/task': patch
---

Make `status` of Task a readonly property

So far `status` was writable which allowed for setting status of task form outside. Doing so did cause rendering of
expected template but the task was becoming internally incoherent.

Now attempt to assign `status` will end up in throwing a `TypeError`.
28 changes: 16 additions & 12 deletions packages/task/src/task.ts
Expand Up @@ -180,7 +180,7 @@ export class Task<
private _abortController?: AbortController;
private _onComplete?: (result: R) => unknown;
private _onError?: (error: unknown) => unknown;
status: TaskStatus = TaskStatus.INITIAL;
private _status: TaskStatus = TaskStatus.INITIAL;

/**
* Determines if the task is run automatically when arguments change after a
Expand All @@ -206,13 +206,13 @@ export class Task<

// Generate an in-progress promise if the the status is pending and has been
// cleared by .run().
if (this.status === TaskStatus.PENDING) {
if (this._status === TaskStatus.PENDING) {
this._taskComplete = new Promise((res, rej) => {
this._resolveTaskComplete = res;
this._rejectTaskComplete = rej;
});
// If the status is error, return a rejected promise.
} else if (this.status === TaskStatus.ERROR) {
} else if (this._status === TaskStatus.ERROR) {
this._taskComplete = Promise.reject(this._error);
// Otherwise we are at a task run's completion or this is the first
// request and we are not in the middle of a task (i.e. INITIAL).
Expand Down Expand Up @@ -251,7 +251,7 @@ export class Task<
// args immediately so it only runs when they change again.
if ('initialValue' in taskConfig) {
this._value = taskConfig.initialValue;
this.status = TaskStatus.COMPLETE;
this._status = TaskStatus.COMPLETE;
this._previousArgs = this._getArgs?.();
}
}
Expand Down Expand Up @@ -318,7 +318,7 @@ export class Task<
// TODO (justinfagnani): add test
this._previousArgs = args;

if (this.status === TaskStatus.PENDING) {
if (this._status === TaskStatus.PENDING) {
this._abortController?.abort();
} else {
// Clear the last complete task run in INITIAL because it may be a resolved
Expand All @@ -329,7 +329,7 @@ export class Task<
this._rejectTaskComplete = undefined;
}

this.status = TaskStatus.PENDING;
this._status = TaskStatus.PENDING;
let result!: R | typeof initialState;
let error: unknown;

Expand All @@ -353,23 +353,23 @@ export class Task<
// If this is the most recent task call, process this value.
if (this._callId === key) {
if (result === initialState) {
this.status = TaskStatus.INITIAL;
this._status = TaskStatus.INITIAL;
} else {
if (errored === false) {
try {
this._onComplete?.(result as R);
} catch {
// Ignore user errors from onComplete.
}
this.status = TaskStatus.COMPLETE;
this._status = TaskStatus.COMPLETE;
this._resolveTaskComplete?.(result as R);
} else {
try {
this._onError?.(error);
} catch {
// Ignore user errors from onError.
}
this.status = TaskStatus.ERROR;
this._status = TaskStatus.ERROR;
this._rejectTaskComplete?.(error);
}
this._value = result as R;
Expand Down Expand Up @@ -399,7 +399,7 @@ export class Task<
* `AbortController.abort()`.
*/
abort(reason?: unknown) {
if (this.status === TaskStatus.PENDING) {
if (this._status === TaskStatus.PENDING) {
this._abortController?.abort(reason);
}
}
Expand All @@ -423,8 +423,12 @@ export class Task<
return this._error;
}

get status() {
return this._status;
}

render<T extends StatusRenderer<R>>(renderer: T) {
switch (this.status) {
switch (this._status) {
case TaskStatus.INITIAL:
return renderer.initial?.() as MaybeReturnType<T['initial']>;
case TaskStatus.PENDING:
Expand All @@ -436,7 +440,7 @@ export class Task<
case TaskStatus.ERROR:
return renderer.error?.(this.error) as MaybeReturnType<T['error']>;
default:
throw new Error(`Unexpected status: ${this.status}`);
throw new Error(`Unexpected status: ${this._status}`);
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions packages/task/src/test/task_test.ts
Expand Up @@ -411,6 +411,18 @@ suite('Task', () => {
assert.equal(el.taskValue, `a1,b`);
});

test('task `status` is not settable', async () => {
const el = getTestElement({args: () => [el.a, el.b], autoRun: false});
await renderElement(el);
await tasksUpdateComplete();
assert.equal(el.task.status, TaskStatus.INITIAL);

assert.throws(() => {
// @ts-expect-error for test
el.task.status = TaskStatus.ERROR;
}, TypeError);
});

test('task runs when `run` called', async () => {
const el = getTestElement({args: () => [el.a, el.b], autoRun: false});
await renderElement(el);
Expand Down