Skip to content

Commit

Permalink
Fix change-in-update warning from Tasks (#3660)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinfagnani committed Feb 10, 2023
1 parent 7e20a52 commit 65df149
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/fast-apes-cross.md
@@ -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
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
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');
});
});

0 comments on commit 65df149

Please sign in to comment.