From 0e0903fd794c592b56f3e2db5f4cae3dea0b2381 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 26 Aug 2021 15:57:10 -0700 Subject: [PATCH 1/4] [ReactiveElement] Adds `scheduleUpdate()` Fixes #2091. Fixes an issue where `performUpdate` was doing double duty as (1) make the element complete a pending update synchronously and (2) override to control update timing. Here `scheduleUpdate()` is added to satisfy (2), reserving (1) for `performUpdate()`. Since by default `scheduleUpdate()` just returns the value of `performUpdate()` this should not break any existing code. However, the docs are updated to recommend that for scheduling `scheduleUpdate()` be implemented in favor of `performUpdate()`. --- .../reactive-element/src/reactive-element.ts | 36 ++++++++++++++----- .../src/test/reactive-element_test.ts | 26 +++++++------- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/packages/reactive-element/src/reactive-element.ts b/packages/reactive-element/src/reactive-element.ts index 1d5d556fed..418a5c5508 100644 --- a/packages/reactive-element/src/reactive-element.ts +++ b/packages/reactive-element/src/reactive-element.ts @@ -1006,8 +1006,8 @@ export abstract class ReactiveElement // `window.onunhandledrejection`. Promise.reject(e); } - const result = this.performUpdate(); - // If `performUpdate` returns a Promise, we await it. This is done to + const result = this.scheduleUpdate(); + // If `scheduleUpdate` returns a Promise, we await it. This is done to // enable coordinating updates with a scheduler. Note, the result is // checked to avoid delaying an additional microtask unless we need to. if (result != null) { @@ -1017,22 +1017,40 @@ export abstract class ReactiveElement } /** - * Performs an element update. Note, if an exception is thrown during the - * update, `firstUpdated` and `updated` will not be called. - * - * You can override this method to change the timing of updates. If this - * method is overridden, `super.performUpdate()` must be called. + * Schedules an element update. You can override this method to change the + * timing of updates. If this method is overridden, `super.scheduleUpdate()` + * must be called. * * For instance, to schedule updates to occur just before the next frame: * * ```ts - * override protected async performUpdate(): Promise { + * override protected async scheduleUpdate(): Promise { * await new Promise((resolve) => requestAnimationFrame(() => resolve())); - * super.performUpdate(); + * super.scheduleUpdate(); * } * ``` * @category updates */ + protected scheduleUpdate(): void | Promise { + return this.performUpdate(); + } + + /** + * Performs an element update. Note, if an exception is thrown during the + * update, `firstUpdated` and `updated` will not be called. + * + * Call performUpdate() to immediately process a pending update. This should + * generally not be needed, but it can be done in rare cases when you need to + * update synchronously. + * + * Note, to ensure `performUpdate()` synchronously completes a pending update, + * it should not be overridden. Previously `performUpdate()` was also used + * for customizing update timing. Instead, use `scheduleUpdate()`. For + * backwards compatibility, scheduling updates via `performUpdate()` + * continues to work. + * + * @category updates + */ protected performUpdate(): void | Promise { // Abort any update if one is not pending when this is called. // This can happen if `performUpdate` is called early to "flush" diff --git a/packages/reactive-element/src/test/reactive-element_test.ts b/packages/reactive-element/src/test/reactive-element_test.ts index 23800b8610..0cb07faf63 100644 --- a/packages/reactive-element/src/test/reactive-element_test.ts +++ b/packages/reactive-element/src/test/reactive-element_test.ts @@ -2331,17 +2331,17 @@ suite('ReactiveElement', () => { assert.equal((el as any).zug, objectValue); }); - test('can override performUpdate()', async () => { + test('can override scheduleUpdate()', async () => { let resolve: ((value?: unknown) => void) | undefined; class A extends ReactiveElement { - performUpdateCalled = false; + scheduleUpdateCalled = false; updateCalled = false; - override async performUpdate() { - this.performUpdateCalled = true; + override async scheduleUpdate() { + this.scheduleUpdateCalled = true; await new Promise((r) => (resolve = r)); - await super.performUpdate(); + await super.scheduleUpdate(); } override update(changedProperties: Map) { @@ -2365,21 +2365,21 @@ suite('ReactiveElement', () => { await new Promise((r) => setTimeout(r, 10)); assert.isFalse(a.updateCalled); - // update is called after performUpdate allowed to complete + // update is called after scheduleUpdate allowed to complete resolve!(); await a.updateComplete; assert.isTrue(a.updateCalled); }); - test('overriding performUpdate() allows nested invalidations', async () => { + test('overriding scheduleUpdate() allows nested invalidations', async () => { class A extends ReactiveElement { - performUpdateCalledCount = 0; + scheduleUpdateCalledCount = 0; updatedCalledCount = 0; - override async performUpdate() { - this.performUpdateCalledCount++; + override async scheduleUpdate() { + this.scheduleUpdateCalledCount++; await new Promise((r) => setTimeout(r)); - super.performUpdate(); + super.scheduleUpdate(); } override updated(_changedProperties: Map) { @@ -2399,14 +2399,14 @@ suite('ReactiveElement', () => { const updateComplete1 = a.updateComplete; await updateComplete1; assert.equal(a.updatedCalledCount, 1); - assert.equal(a.performUpdateCalledCount, 1); + assert.equal(a.scheduleUpdateCalledCount, 1); const updateComplete2 = a.updateComplete; assert.notStrictEqual(updateComplete1, updateComplete2); await updateComplete2; assert.equal(a.updatedCalledCount, 2); - assert.equal(a.performUpdateCalledCount, 2); + assert.equal(a.scheduleUpdateCalledCount, 2); }); test('update does not occur before element is connected', async () => { From 7e6e2d6fe236b578a919809e24d89ae1606a6bd0 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 26 Aug 2021 16:00:11 -0700 Subject: [PATCH 2/4] Add changeset. --- .changeset/clean-dancers-report.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/clean-dancers-report.md diff --git a/.changeset/clean-dancers-report.md b/.changeset/clean-dancers-report.md new file mode 100644 index 0000000000..c21ef7302c --- /dev/null +++ b/.changeset/clean-dancers-report.md @@ -0,0 +1,5 @@ +--- +'@lit/reactive-element': patch +--- + +Adds `scheduleUpdate()` to control update timing. This should be implemented instead of `performUpdate()`; however, existing overrides of `performUpdate()` will continue to work. From 9d9bc676361487101b5b18d9ae2516a04570b431 Mon Sep 17 00:00:00 2001 From: Steven Orvell Date: Thu, 26 Aug 2021 16:58:36 -0700 Subject: [PATCH 3/4] Address review feedback. --- packages/reactive-element/src/reactive-element.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/reactive-element/src/reactive-element.ts b/packages/reactive-element/src/reactive-element.ts index 418a5c5508..0638b3b54e 100644 --- a/packages/reactive-element/src/reactive-element.ts +++ b/packages/reactive-element/src/reactive-element.ts @@ -1018,7 +1018,9 @@ export abstract class ReactiveElement /** * Schedules an element update. You can override this method to change the - * timing of updates. If this method is overridden, `super.scheduleUpdate()` + * timing of updates by returning a Promise. The update will await the + * returned Promise, and you should resolve the Promise to allow the update + * to proceed. If this method is overridden, `super.scheduleUpdate()` * must be called. * * For instance, to schedule updates to occur just before the next frame: From b2beae2fd3fef364b0bb55e74a696e1c759d7fb2 Mon Sep 17 00:00:00 2001 From: Steve Orvell Date: Thu, 26 Aug 2021 17:13:19 -0700 Subject: [PATCH 4/4] Update packages/reactive-element/src/reactive-element.ts Co-authored-by: Justin Fagnani --- packages/reactive-element/src/reactive-element.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/reactive-element/src/reactive-element.ts b/packages/reactive-element/src/reactive-element.ts index 0638b3b54e..704ceffb82 100644 --- a/packages/reactive-element/src/reactive-element.ts +++ b/packages/reactive-element/src/reactive-element.ts @@ -1045,11 +1045,12 @@ export abstract class ReactiveElement * generally not be needed, but it can be done in rare cases when you need to * update synchronously. * - * Note, to ensure `performUpdate()` synchronously completes a pending update, - * it should not be overridden. Previously `performUpdate()` was also used - * for customizing update timing. Instead, use `scheduleUpdate()`. For - * backwards compatibility, scheduling updates via `performUpdate()` - * continues to work. + * Note: To ensure `performUpdate()` synchronously completes a pending update, + * it should not be overridden. In LitElement 2.x it was suggested to override + * `performUpdate()` to also customizing update scheduling. Instead, you should now + * override `scheduleUpdate()`. For backwards compatibility with LitElement 2.x, + * scheduling updates via `performUpdate()` continues to work, but will make + * also calling `performUpdate()` to synchronously process updates difficult. * * @category updates */