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

Fix change-in-update warning from Tasks #3660

Merged
merged 3 commits into from
Feb 10, 2023
Merged
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
5 changes: 5 additions & 0 deletions .changeset/fast-apes-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lit-labs/task': minor
---

Fix the change-in-update warning from Tasks by delaying the initial host update
9 changes: 6 additions & 3 deletions packages/labs/task/src/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const TaskStatus = {
*/
export const initialState = Symbol();

export type TaskStatus = typeof TaskStatus[keyof typeof TaskStatus];
export type TaskStatus = (typeof TaskStatus)[keyof typeof TaskStatus];

export type StatusRenderer<R> = {
initial?: () => unknown;
Expand Down Expand Up @@ -203,8 +203,11 @@ export class Task<
this.status = TaskStatus.PENDING;
let result!: R | typeof initialState;
let error: unknown;
// Request an update to report pending state.
this._host.requestUpdate();

// Request an update to report pending state. Do this in a
// microtask to avoid the change-in-update warning
queueMicrotask(() => this._host.requestUpdate());

const key = ++this._callId;
try {
result = await this._task(args!);
Expand Down
70 changes: 62 additions & 8 deletions packages/labs/task/src/test/task_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ import {initialState, Task, TaskStatus, TaskConfig} from '@lit-labs/task';
import {generateElementName, nextFrame} from './test-helpers.js';
import {assert} from '@esm-bundle/chai';

// Note, since tests are not built with production support, detect DEV_MODE
// by checking if warning API is available.
const DEV_MODE = !!ReactiveElement.enableWarning;

if (DEV_MODE) {
ReactiveElement.disableWarning?.('change-in-update');
}

suite('Task', () => {
let container: HTMLElement;

Expand Down Expand Up @@ -92,9 +84,29 @@ suite('Task', () => {

const tasksUpdateComplete = nextFrame;

let warnMessages: Array<string>;
const originalConsoleWarn = console.warn;

suiteSetup(() => {
// Patch console.warn to check warnings
console.warn = (...args) => {
warnMessages.push(args.join(' '));
return originalConsoleWarn.apply(console, args);
};
});

suiteTeardown(() => {
// Un-patch console.warn
console.warn = originalConsoleWarn;
});

setup(async () => {
container = document.createElement('div');
document.body.appendChild(container);
warnMessages = [];

// Individual tests can enable the warning
// ReactiveElement.disableWarning?.('change-in-update');
});

teardown(() => {
Expand Down Expand Up @@ -528,4 +540,46 @@ suite('Task', () => {
assert.equal(numOnErrorInvocations, 1);
assert.equal(lastOnErrorResult, 'error2');
});

test('no change-in-update warning', async () => {
ReactiveElement.enableWarning?.('change-in-update');
let numInvocations = 0;

const el = getTestElement({
args: () => [1],
onComplete: () => {
numInvocations++;
},
});
await renderElement(el);
el.resolveTask();
await tasksUpdateComplete();
assert.equal(numInvocations, 1);
assert.equal(warnMessages.length, 0);
});

test('Tasks can see effects of update()', async () => {
class TestElement extends ReactiveElement {
task = new Task(this, {
args: () => [],
task: () => {
this.taskObservedValue = this.value;
},
});
value = 'foo';
taskObservedValue: string | undefined = undefined;

override update(changedProps: PropertyValues) {
super.update(changedProps);
this.value = 'bar';
}
}
customElements.define(generateElementName(), TestElement);
const el = new TestElement();
container.appendChild(el);
await el.updateComplete;
await el.task.taskComplete;

assert.equal(el.taskObservedValue, 'bar');
});
});