From b51c91cec61da0920529fcc321e081a34f798d48 Mon Sep 17 00:00:00 2001 From: jorenbroekema Date: Tue, 29 Jun 2021 15:20:01 +0200 Subject: [PATCH] feat: support multiple pending tasks and recovering from error --- .changeset/new-rules-wait.md | 5 + README.md | 40 ++++++- demo/DemoElement.js | 19 +++- index.js | 1 + package.json | 4 +- src/ResetErrorEvent.js | 5 + src/SuspenseElement.js | 76 +++++++++++-- test/SuspenseElement.test.js | 158 +++++++++++++++++++++++++++- web-test-runner-chromium.config.mjs | 7 ++ web-test-runner-firefox.config.mjs | 7 ++ web-test-runner-webkit.config.mjs | 7 ++ 11 files changed, 316 insertions(+), 13 deletions(-) create mode 100644 .changeset/new-rules-wait.md create mode 100644 src/ResetErrorEvent.js create mode 100644 web-test-runner-chromium.config.mjs create mode 100644 web-test-runner-firefox.config.mjs create mode 100644 web-test-runner-webkit.config.mjs diff --git a/.changeset/new-rules-wait.md b/.changeset/new-rules-wait.md new file mode 100644 index 0000000..1fd04bd --- /dev/null +++ b/.changeset/new-rules-wait.md @@ -0,0 +1,5 @@ +--- +'suspense-element': minor +--- + +Support multiple pending tasks sent in separate events. Support going back to pending state after success. Support going back to pending state after error, by sending a ResetErrorEvent to the suspense-element. diff --git a/README.md b/README.md index 9bccd30..bc70ca9 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,45 @@ class MainElement extends HTMLElement { } ``` -> With a Lit component you would probably set shouldUpdate to false until all data is available that the template depends on and trigger a manual re-render. +### Demo subsequent pending tasks + +If you're viewing the docs site, below is a demo of the suspense-element where the main element fires pending tasks in 3 second intervals, each resolving in 1 second. It switches between resolving and rejecting. + + + Loading... + Error :( + + + +#### ResetErrorEvent + +When sending multiple pending tasks, either in a single event, stacking multiple, or in subsequent (unstacked) pending tasks, `suspense-element` has to decide what to do when any of these tasks throw. +It will display the error slot if it encounters any error. +It will keep doing so even if all pending tasks have completed (some threw), and you send a new one completely separately. +The reason for this behavior is that when your main element depends on asynchronous tasks, and one of them throws at any point, new pending tasks do not mean a recovery from old errors even if the new task resolves, so it makes more sense to maintain the error state. + +If you need to recover from this you can do so manually, by sending a ResetErrorEvent to the `suspense-element`. +This will reset the internal error state and re-evaluate if there are any pending tasks, if so, render the fallback slot. + +```js +import { ResetErrorEvent } from 'suspense-element'; + +class MainElement extends HTMLElement { + constructor() { + super(); + this.attachShadow({ mode: 'open' }); + this.dispatchEvent(new PendingTaskEvent( + new Promise((resolve, reject) => setTimeout(reject, 100)), + )); + + setTimeout(() => { + this.dispatchEvent(new ResetErrorEvent()); + // error slot is still displayed, but sending a new pending task event + // will set the state to 'pending' and display fallback slot. + }, 110); + } +} +``` ## Rationale diff --git a/demo/DemoElement.js b/demo/DemoElement.js index 627cc91..3690456 100644 --- a/demo/DemoElement.js +++ b/demo/DemoElement.js @@ -1,4 +1,4 @@ -import { PendingTaskEvent } from '../src/PendingTaskEvent.js'; +import { PendingTaskEvent, ResetErrorEvent } from '../index.js'; export class DemoElement extends HTMLElement { constructor() { @@ -6,6 +6,7 @@ export class DemoElement extends HTMLElement { this.attachShadow({ mode: 'open' }); /** @type {string[]} */ this.listData = []; + this.resolve = false; } connectedCallback() { @@ -18,6 +19,22 @@ export class DemoElement extends HTMLElement { }, 1000), ); this.dispatchEvent(new PendingTaskEvent(this.list)); + + if (this.hasAttribute('pending-interval')) { + setInterval(() => { + this.dispatchEvent(new ResetErrorEvent()); + this.dispatchEvent( + new PendingTaskEvent( + new Promise((resolve, reject) => + setTimeout(() => { + this.resolve ? resolve() : reject(); + this.resolve = !this.resolve; + }, 1000), + ), + ), + ); + }, 3000); + } } render() { diff --git a/index.js b/index.js index 5508fee..7a696fb 100644 --- a/index.js +++ b/index.js @@ -1,2 +1,3 @@ export { SuspenseElement } from './src/SuspenseElement.js'; export { PendingTaskEvent } from './src/PendingTaskEvent.js'; +export { ResetErrorEvent } from './src/ResetErrorEvent.js'; diff --git a/package.json b/package.json index ef0a792..e90e5d3 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,9 @@ "scripts": { "build": "npm run custom-elements-manifest && tsc -p tsconfig.build.types.json && npm run demo:build", "custom-elements-manifest": "custom-elements-manifest analyze", - "debug": "wtr test/**/*.test.js --watch", + "debug": "wtr test/**/*.test.js --watch --config web-test-runner-chromium.config.mjs", + "debug:firefox": "wtr test/**/*.test.js --watch --config web-test-runner-firefox.config.mjs", + "debug:webkit": "wtr test/**/*.test.js --watch --config web-test-runner-webkit.config.mjs", "demo:build": "node marked/run-marked.js", "demo:prod": "npm run demo:build && rollup -c rollup.config.js", "demo:watch": "node marked/watch-marked.js", diff --git a/src/ResetErrorEvent.js b/src/ResetErrorEvent.js new file mode 100644 index 0000000..7c33753 --- /dev/null +++ b/src/ResetErrorEvent.js @@ -0,0 +1,5 @@ +export class ResetErrorEvent extends Event { + constructor() { + super('reset-error', { bubbles: true, composed: true }); + } +} diff --git a/src/SuspenseElement.js b/src/SuspenseElement.js index 91e951e..963adb5 100644 --- a/src/SuspenseElement.js +++ b/src/SuspenseElement.js @@ -1,5 +1,6 @@ /** * @typedef {import('./PendingTaskEvent.js').PendingTaskEvent} PendingTaskEvent + * @typedef {import('./ResetErrorEvent.js').ResetErrorEvent} ResetErrorEvent */ export class SuspenseElement extends HTMLElement { get state() { @@ -11,12 +12,27 @@ export class SuspenseElement extends HTMLElement { this.setAttribute('state', value); } + get pendingTaskCount() { + return /** @type {number} */ (this._pendingTaskCount); + } + + /** @param {number} value */ + set pendingTaskCount(value) { + this._pendingTaskCount = value; + + this._handleState(); + } + constructor() { super(); + this._pendingTaskCount = 0; this.state = 'pending'; + this._errorState = false; this.attachShadow({ mode: 'open' }); this.boundPendingTaskEventHandler = this.pendingTaskEventHandler.bind(this); + this.boundResetErrorHandler = this.resetErrorHandler.bind(this); this.addEventListener('pending-task', this.boundPendingTaskEventHandler); + this.addEventListener('reset-error', this.boundResetErrorHandler); this.render(); } @@ -49,21 +65,63 @@ export class SuspenseElement extends HTMLElement { } } + /** @param {ResetErrorEvent} e */ + resetErrorHandler(e) { + e.stopPropagation(); + this._errorState = false; + if (this.pendingTaskCount > 0) { + this._setPending(); + } + } + /** @param {Event} e */ async pendingTaskEventHandler(e) { const _e = /** @type {PendingTaskEvent} */ (e); _e.stopPropagation(); - + this.pendingTaskCount += 1; _e.complete - .then(() => { - this.state = 'success'; - this.style.setProperty('--main-display', 'block'); - this.style.setProperty('--fallback-display', 'none'); - }) .catch(() => { - this.state = 'error'; - this.style.setProperty('--error-display', 'block'); - this.style.setProperty('--fallback-display', 'none'); + this._errorState = true; + }) + .finally(() => { + this.pendingTaskCount -= 1; }); } + + _handleState() { + if (this._errorState) { + this._setError(); + return; + } + + if (this.pendingTaskCount > 0 && this.state !== 'pending') { + this._setPending(); + return; + } + + if (this.pendingTaskCount === 0) { + this._setSuccess(); + } + } + + _setPending() { + this.state = 'pending'; + this.style.setProperty('--error-display', 'none'); + this.style.setProperty('--fallback-display', 'block'); + this.style.setProperty('--main-display', 'none'); + } + + _setError() { + this.state = 'error'; + this.style.setProperty('--error-display', 'block'); + this.style.setProperty('--fallback-display', 'none'); + this.style.setProperty('--main-display', 'none'); + } + + _setSuccess() { + this.state = 'success'; + this.style.setProperty('--error-display', 'none'); + this.style.setProperty('--fallback-display', 'none'); + this.style.setProperty('--main-display', 'block'); + } } diff --git a/test/SuspenseElement.test.js b/test/SuspenseElement.test.js index 6deeab8..7660a7a 100644 --- a/test/SuspenseElement.test.js +++ b/test/SuspenseElement.test.js @@ -1,5 +1,5 @@ import { expect, fixture as _fixture, defineCE, aTimeout } from '@open-wc/testing'; -import { PendingTaskEvent } from '../src/PendingTaskEvent.js'; +import { PendingTaskEvent, ResetErrorEvent } from '../index.js'; import '../define.js'; /** @@ -152,4 +152,160 @@ describe('', () => { expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); }); + + it('handles subsequent async tasks through pending-task events, showing again the fallback until it resolves', async () => { + const el = await fixture(` + + Loading... + Error :( + <${mainTag}> + + `); + + const { mainSlottable, fallbackSlot, errorSlot, mainSlot } = getMembers(el); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + }); + + it('handles multiple stacked pending tasks and resolves to rendering the main element after all of them have resolved', async () => { + const el = await fixture(` + + Loading... + Error :( + <${mainTag}> + + `); + + const { mainSlottable, fallbackSlot, errorSlot, mainSlot } = getMembers(el); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 150))), + ); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + await aTimeout(150); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + }); + + it('keeps showing the error slot by default when a single pending task rejects', async () => { + const el = await fixture(` + + Loading... + Error :( + <${mainTag} reject> + + `); + + const { mainSlottable, fallbackSlot, errorSlot, mainSlot } = getMembers(el); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + }); + + it('supports resetting the error state in order for pending and success to be possible again', async () => { + const el = await fixture(` + + Loading... + Error :( + <${mainTag} reject> + + `); + + const { mainSlottable, fallbackSlot, errorSlot, mainSlot } = getMembers(el); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + mainSlottable.dispatchEvent(new ResetErrorEvent()); + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + }); + + it('supports resetting the error state while a pending task was already sent and is still pending', async () => { + const el = await fixture(` + + Loading... + Error :( + <${mainTag} reject> + + `); + + const { mainSlottable, fallbackSlot, errorSlot, mainSlot } = getMembers(el); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + mainSlottable.dispatchEvent( + new PendingTaskEvent(new Promise((resolve) => setTimeout(resolve, 50))), + ); + mainSlottable.dispatchEvent(new ResetErrorEvent()); + + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`block`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`none`); + + await aTimeout(50); + expect(getComputedStyle(errorSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(fallbackSlot).getPropertyValue('display')).to.equal(`none`); + expect(getComputedStyle(mainSlot).getPropertyValue('display')).to.equal(`block`); + }); }); diff --git a/web-test-runner-chromium.config.mjs b/web-test-runner-chromium.config.mjs new file mode 100644 index 0000000..38710a6 --- /dev/null +++ b/web-test-runner-chromium.config.mjs @@ -0,0 +1,7 @@ +import { playwrightLauncher } from '@web/test-runner-playwright'; +import baseConfig from './web-test-runner.config.mjs'; + +export default { + ...baseConfig, + browsers: [playwrightLauncher({ product: 'chromium' })], +}; diff --git a/web-test-runner-firefox.config.mjs b/web-test-runner-firefox.config.mjs new file mode 100644 index 0000000..60ce90a --- /dev/null +++ b/web-test-runner-firefox.config.mjs @@ -0,0 +1,7 @@ +import { playwrightLauncher } from '@web/test-runner-playwright'; +import baseConfig from './web-test-runner.config.mjs'; + +export default { + ...baseConfig, + browsers: [playwrightLauncher({ product: 'firefox', concurrency: 1 })], +}; diff --git a/web-test-runner-webkit.config.mjs b/web-test-runner-webkit.config.mjs new file mode 100644 index 0000000..5de663e --- /dev/null +++ b/web-test-runner-webkit.config.mjs @@ -0,0 +1,7 @@ +import { playwrightLauncher } from '@web/test-runner-playwright'; +import baseConfig from './web-test-runner.config.mjs'; + +export default { + ...baseConfig, + browsers: [playwrightLauncher({ product: 'webkit' })], +};