From 2fb836c63774ce7eb7b65adfbfe49a613797fa42 Mon Sep 17 00:00:00 2001 From: Lucas Garron Date: Thu, 27 Oct 2022 17:46:59 -0700 Subject: [PATCH 1/5] Switch `Promise` chaining to `async`-`await` syntax. This will make future PRs like https://github.com/github/include-fragment-element/pull/81 easier to author and review. --- src/index.ts | 109 ++++++++++++++++++++++++--------------------------- 1 file changed, 52 insertions(+), 57 deletions(-) diff --git a/src/index.ts b/src/index.ts index 86c16ba..940bac3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -133,28 +133,28 @@ export default class IncludeFragmentElement extends HTMLElement { } ) - #handleData(): Promise { + async #handleData(): Promise { if (this.#busy) return Promise.resolve() this.#busy = true - - this.#observer.unobserve(this) - return this.#getData().then( - (html: string) => { - const template = document.createElement('template') - // eslint-disable-next-line github/no-inner-html - template.innerHTML = html - const fragment = document.importNode(template.content, true) - const canceled = !this.dispatchEvent( - new CustomEvent('include-fragment-replace', {cancelable: true, detail: {fragment}}) - ) - if (canceled) return - this.replaceWith(fragment) - this.dispatchEvent(new CustomEvent('include-fragment-replaced')) - }, - () => { - this.classList.add('is-error') + try { + this.#observer.unobserve(this) + const html = await this.#getData() + + const template = document.createElement('template') + // eslint-disable-next-line github/no-inner-html + template.innerHTML = html + const fragment = document.importNode(template.content, true) + const canceled = !this.dispatchEvent( + new CustomEvent('include-fragment-replace', {cancelable: true, detail: {fragment}}) + ) + if (canceled) { + return } - ) + this.replaceWith(fragment) + this.dispatchEvent(new CustomEvent('include-fragment-replaced')) + } catch (_) { + this.classList.add('is-error') + } } #getData(): Promise { @@ -173,47 +173,42 @@ export default class IncludeFragmentElement extends HTMLElement { } } - #fetchDataWithEvents(): Promise { + async #fetchDataWithEvents(): Promise { // We mimic the same event order as , including the spec // which states events must be dispatched after "queue a task". // https://www.w3.org/TR/html52/semantics-embedded-content.html#the-img-element - return task() - .then(() => { - this.dispatchEvent(new Event('loadstart')) - return this.fetch(this.request()) - }) - .then(response => { - if (response.status !== 200) { - throw new Error(`Failed to load resource: the server responded with a status of ${response.status}`) - } - const ct = response.headers.get('Content-Type') - if (!isWildcard(this.accept) && (!ct || !ct.includes(this.accept ? this.accept : 'text/html'))) { - throw new Error(`Failed to load resource: expected ${this.accept || 'text/html'} but was ${ct}`) - } - return response.text() - }) - .then( - data => { - // Dispatch `load` and `loadend` async to allow - // the `load()` promise to resolve _before_ these - // events are fired. - task().then(() => { - this.dispatchEvent(new Event('load')) - this.dispatchEvent(new Event('loadend')) - }) - return data - }, - error => { - // Dispatch `error` and `loadend` async to allow - // the `load()` promise to resolve _before_ these - // events are fired. - task().then(() => { - this.dispatchEvent(new Event('error')) - this.dispatchEvent(new Event('loadend')) - }) - throw error - } - ) + + try { + await task() + + this.dispatchEvent(new Event('loadstart')) + const response = await this.fetch(this.request()) + + if (response.status !== 200) { + throw new Error(`Failed to load resource: the server responded with a status of ${response.status}`) + } + const ct = response.headers.get('Content-Type') + if (!isWildcard(this.accept) && (!ct || !ct.includes(this.accept ? this.accept : 'text/html'))) { + throw new Error(`Failed to load resource: expected ${this.accept || 'text/html'} but was ${ct}`) + } + const data = await response.text() + + // Dispatch `load` and `loadend` async to allow + // the `load()` promise to resolve _before_ these + // events are fired. + await task() + this.dispatchEvent(new Event('load')) + this.dispatchEvent(new Event('loadend')) + return data + } catch (error) { + // Dispatch `error` and `loadend` async to allow + // the `load()` promise to resolve _before_ these + // events are fired. + await task() + this.dispatchEvent(new Event('error')) + this.dispatchEvent(new Event('loadend')) + throw error + } } } From 4b6a263769f13d0c549d822fa717d7ae68175bc6 Mon Sep 17 00:00:00 2001 From: Lucas Garron Date: Thu, 27 Oct 2022 17:49:51 -0700 Subject: [PATCH 2/5] Consolidate task calls. --- src/index.ts | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/index.ts b/src/index.ts index 940bac3..ccf8a13 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,10 +1,5 @@ const privateData = new WeakMap() -// Functional stand in for the W3 spec "queue a task" paradigm -function task(): Promise { - return new Promise(resolve => setTimeout(resolve, 0)) -} - function isWildcard(accept: string | null) { return accept && !!accept.split(',').find(x => x.match(/^\s*\*\/\*/)) } @@ -173,17 +168,20 @@ export default class IncludeFragmentElement extends HTMLElement { } } + // Functional stand in for the W3 spec "queue a task" paradigm + async #task(eventsToDispatch: string[]): Promise { + await new Promise(resolve => setTimeout(resolve, 0)) + eventsToDispatch.forEach(eventType => this.dispatchEvent(new Event(eventType))) + } + async #fetchDataWithEvents(): Promise { // We mimic the same event order as , including the spec // which states events must be dispatched after "queue a task". // https://www.w3.org/TR/html52/semantics-embedded-content.html#the-img-element try { - await task() - - this.dispatchEvent(new Event('loadstart')) + await this.#task(['loadstart']) const response = await this.fetch(this.request()) - if (response.status !== 200) { throw new Error(`Failed to load resource: the server responded with a status of ${response.status}`) } @@ -196,17 +194,13 @@ export default class IncludeFragmentElement extends HTMLElement { // Dispatch `load` and `loadend` async to allow // the `load()` promise to resolve _before_ these // events are fired. - await task() - this.dispatchEvent(new Event('load')) - this.dispatchEvent(new Event('loadend')) + await this.#task(['load', 'loadend']) return data } catch (error) { // Dispatch `error` and `loadend` async to allow // the `load()` promise to resolve _before_ these // events are fired. - await task() - this.dispatchEvent(new Event('error')) - this.dispatchEvent(new Event('loadend')) + await this.#task(['error', 'loadend']) throw error } } From d7f560c0441a1856ce9afa7b52bb08e06e9c4d42 Mon Sep 17 00:00:00 2001 From: Lucas Garron Date: Thu, 27 Oct 2022 18:01:11 -0700 Subject: [PATCH 3/5] Switch to `for`-`of` per the linter. --- src/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index ccf8a13..3b105cf 100644 --- a/src/index.ts +++ b/src/index.ts @@ -171,7 +171,9 @@ export default class IncludeFragmentElement extends HTMLElement { // Functional stand in for the W3 spec "queue a task" paradigm async #task(eventsToDispatch: string[]): Promise { await new Promise(resolve => setTimeout(resolve, 0)) - eventsToDispatch.forEach(eventType => this.dispatchEvent(new Event(eventType))) + for (const eventType of eventsToDispatch) { + this.dispatchEvent(new Event(eventType)) + } } async #fetchDataWithEvents(): Promise { From fcff3982149b86a5404c3d7507ecfc82aa6e5824 Mon Sep 17 00:00:00 2001 From: Lucas Garron Date: Thu, 27 Oct 2022 18:07:55 -0700 Subject: [PATCH 4/5] Fixups. --- src/index.ts | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/index.ts b/src/index.ts index 3b105cf..3bab195 100644 --- a/src/index.ts +++ b/src/index.ts @@ -129,10 +129,10 @@ export default class IncludeFragmentElement extends HTMLElement { ) async #handleData(): Promise { - if (this.#busy) return Promise.resolve() + if (this.#busy) return this.#busy = true + this.#observer.unobserve(this) try { - this.#observer.unobserve(this) const html = await this.#getData() const template = document.createElement('template') @@ -142,9 +142,7 @@ export default class IncludeFragmentElement extends HTMLElement { const canceled = !this.dispatchEvent( new CustomEvent('include-fragment-replace', {cancelable: true, detail: {fragment}}) ) - if (canceled) { - return - } + if (canceled) return this.replaceWith(fragment) this.dispatchEvent(new CustomEvent('include-fragment-replaced')) } catch (_) { @@ -181,28 +179,28 @@ export default class IncludeFragmentElement extends HTMLElement { // which states events must be dispatched after "queue a task". // https://www.w3.org/TR/html52/semantics-embedded-content.html#the-img-element - try { - await this.#task(['loadstart']) - const response = await this.fetch(this.request()) - if (response.status !== 200) { - throw new Error(`Failed to load resource: the server responded with a status of ${response.status}`) - } - const ct = response.headers.get('Content-Type') - if (!isWildcard(this.accept) && (!ct || !ct.includes(this.accept ? this.accept : 'text/html'))) { - throw new Error(`Failed to load resource: expected ${this.accept || 'text/html'} but was ${ct}`) - } - const data = await response.text() + await this.#task(['loadstart']) + const response = await this.fetch(this.request()) + if (response.status !== 200) { + throw new Error(`Failed to load resource: the server responded with a status of ${response.status}`) + } + const ct = response.headers.get('Content-Type') + if (!isWildcard(this.accept) && (!ct || !ct.includes(this.accept ? this.accept : 'text/html'))) { + throw new Error(`Failed to load resource: expected ${this.accept || 'text/html'} but was ${ct}`) + } + const data = await response.text() + try { // Dispatch `load` and `loadend` async to allow // the `load()` promise to resolve _before_ these // events are fired. - await this.#task(['load', 'loadend']) + this.#task(['load', 'loadend']) return data } catch (error) { // Dispatch `error` and `loadend` async to allow // the `load()` promise to resolve _before_ these // events are fired. - await this.#task(['error', 'loadend']) + this.#task(['error', 'loadend']) throw error } } From d3f7b4d79dff63f0ba27d55f6c8116a4fed71a17 Mon Sep 17 00:00:00 2001 From: Lucas Garron Date: Mon, 31 Oct 2022 11:06:58 -0700 Subject: [PATCH 5/5] Update src/index.ts Co-authored-by: David Graham --- src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 3bab195..be8cdd4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -145,7 +145,7 @@ export default class IncludeFragmentElement extends HTMLElement { if (canceled) return this.replaceWith(fragment) this.dispatchEvent(new CustomEvent('include-fragment-replaced')) - } catch (_) { + } catch { this.classList.add('is-error') } }