From 2cb6b91cb35fe024fead063cf23c7dfab5c67acc Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 10 Sep 2019 11:36:34 -0400 Subject: [PATCH 1/6] Add test to expect failure handling --- test/karma.config.js | 4 +++- test/test.js | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/test/karma.config.js b/test/karma.config.js index edf3501..081037b 100644 --- a/test/karma.config.js +++ b/test/karma.config.js @@ -1,6 +1,8 @@ function reply(request, response, next) { if (request.method === 'GET') { - response.writeHead(200) + const status = request.url.startsWith('/500') ? 500 : 200 + response.writeHead(status) + response.ok = status === 200 response.end(`
  1. item
    2. diff --git a/test/test.js b/test/test.js index 00b6ef0..9edc583 100644 --- a/test/test.js +++ b/test/test.js @@ -38,6 +38,25 @@ describe('remote-input', function() { input.focus() }) + it('handles not ok responses', function(done) { + const remoteInput = document.querySelector('remote-input') + const input = document.querySelector('input') + const results = document.querySelector('#results') + remoteInput.src = '/500' + assert.equal(results.innerHTML, '') + let errorHappened = false + remoteInput.addEventListener('error', function() { + errorHappened = true + }) + remoteInput.addEventListener('loadend', function() { + assert.ok(errorHappened, 'error event happened') + assert.equal(results.innerHTML, '', 'nothing was appended') + done() + }) + input.value = 'test' + input.focus() + }) + it('repects param attribute', function(done) { const remoteInput = document.querySelector('remote-input') const input = document.querySelector('input') From 8f5ed49e2a9f2b8a9172decc7210c2457907ce95 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 10 Sep 2019 11:37:59 -0400 Subject: [PATCH 2/6] Only append results if response.ok --- index.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 16a1227..50a6b23 100644 --- a/index.js +++ b/index.js @@ -70,17 +70,23 @@ async function fetchResults(remoteInput: RemoteInputElement, checkCurrentQuery: remoteInput.dispatchEvent(new CustomEvent('loadstart')) remoteInput.setAttribute('loading', '') + let response, html try { - const response = await fetch(url, { + response = await fetch(url, { credentials: 'same-origin', headers: {accept: 'text/html; fragment'} }) - const html = await response.text() + html = await response.text() remoteInput.dispatchEvent(new CustomEvent('load')) - resultsContainer.innerHTML = html } catch { + // noop + } + if (response && response.ok && html) { + resultsContainer.innerHTML = html + } else { remoteInput.dispatchEvent(new CustomEvent('error')) } + remoteInput.removeAttribute('loading') remoteInput.dispatchEvent(new CustomEvent('loadend')) } From e2e2dc12c02c4f419aa2a1d50da0e05f3626538c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mu-An=20=E6=85=95=E5=AE=89?= Date: Tue, 10 Sep 2019 12:23:09 -0400 Subject: [PATCH 3/6] Apply suggestions from code review empty html should be accepted Co-Authored-By: David Graham --- index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 50a6b23..46e1025 100644 --- a/index.js +++ b/index.js @@ -70,7 +70,8 @@ async function fetchResults(remoteInput: RemoteInputElement, checkCurrentQuery: remoteInput.dispatchEvent(new CustomEvent('loadstart')) remoteInput.setAttribute('loading', '') - let response, html + let response + let html = '' try { response = await fetch(url, { credentials: 'same-origin', @@ -79,9 +80,9 @@ async function fetchResults(remoteInput: RemoteInputElement, checkCurrentQuery: html = await response.text() remoteInput.dispatchEvent(new CustomEvent('load')) } catch { - // noop + // Network errors handled below. } - if (response && response.ok && html) { + if (response && response.ok) { resultsContainer.innerHTML = html } else { remoteInput.dispatchEvent(new CustomEvent('error')) From bf97eec977f3405d51cbf3a5ea67ce63a33f022e Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 10 Sep 2019 14:37:32 -0400 Subject: [PATCH 4/6] Introduct custom events to not overload network error events --- index.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 46e1025..45e12d6 100644 --- a/index.js +++ b/index.js @@ -71,6 +71,7 @@ async function fetchResults(remoteInput: RemoteInputElement, checkCurrentQuery: remoteInput.dispatchEvent(new CustomEvent('loadstart')) remoteInput.setAttribute('loading', '') let response + let errored = false let html = '' try { response = await fetch(url, { @@ -80,15 +81,19 @@ async function fetchResults(remoteInput: RemoteInputElement, checkCurrentQuery: html = await response.text() remoteInput.dispatchEvent(new CustomEvent('load')) } catch { - // Network errors handled below. + errored = true + remoteInput.dispatchEvent(new CustomEvent('error')) } + remoteInput.removeAttribute('loading') + if (errored) return + if (response && response.ok) { + remoteInput.dispatchEvent(new CustomEvent('remote-input-success', {bubbles: true})) resultsContainer.innerHTML = html } else { - remoteInput.dispatchEvent(new CustomEvent('error')) + remoteInput.dispatchEvent(new CustomEvent('remote-input-error', {bubbles: true})) } - remoteInput.removeAttribute('loading') remoteInput.dispatchEvent(new CustomEvent('loadend')) } From 6e8ae1ef833639986c16202e515270b072ed1b40 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 10 Sep 2019 14:58:18 -0400 Subject: [PATCH 5/6] Add test for custom and network error events --- test/karma.config.js | 28 ++++++++++++++++++---------- test/test.js | 30 ++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/test/karma.config.js b/test/karma.config.js index 081037b..8c2620a 100644 --- a/test/karma.config.js +++ b/test/karma.config.js @@ -1,15 +1,23 @@ function reply(request, response, next) { if (request.method === 'GET') { - const status = request.url.startsWith('/500') ? 500 : 200 - response.writeHead(status) - response.ok = status === 200 - response.end(` -
      -
    1. item
      2. -
    2. item
      3. -
    3. item
      4. -
    - `) + if (request.url.startsWith('/500')) { + response.writeHead(500) + response.ok = false + response.end('Server error') + } else if (request.url.startsWith('/network-error')) { + request.destroy(new Error()) + response.end() + } else { + response.writeHead(200) + response.ok = true + response.end(` +
      +
    1. item
      2. +
    2. item
      3. +
    3. item
      4. +
    + `) + } return } next() diff --git a/test/test.js b/test/test.js index 9edc583..81b2e79 100644 --- a/test/test.js +++ b/test/test.js @@ -30,7 +30,12 @@ describe('remote-input', function() { const input = document.querySelector('input') const results = document.querySelector('#results') assert.equal(results.innerHTML, '') + let successEvent = false + remoteInput.addEventListener('remote-input-success', function() { + successEvent = true + }) remoteInput.addEventListener('loadend', function() { + assert.ok(successEvent, 'success event happened') assert.equal(results.querySelector('ol').getAttribute('data-src'), '/results?q=test') done() }) @@ -44,17 +49,34 @@ describe('remote-input', function() { const results = document.querySelector('#results') remoteInput.src = '/500' assert.equal(results.innerHTML, '') - let errorHappened = false - remoteInput.addEventListener('error', function() { - errorHappened = true + let errorEvent = false + remoteInput.addEventListener('remote-input-error', function() { + errorEvent = true }) remoteInput.addEventListener('loadend', function() { - assert.ok(errorHappened, 'error event happened') + assert.ok(errorEvent, 'error event happened') + assert.equal(results.innerHTML, '', 'nothing was appended') + done() + }) + input.value = 'test' + input.focus() + }) + + it('handles network error', function(done) { + const remoteInput = document.querySelector('remote-input') + const input = document.querySelector('input') + const results = document.querySelector('#results') + remoteInput.src = '/network-error' + assert.equal(results.innerHTML, '') + remoteInput.addEventListener('error', async function() { + await Promise.resolve() assert.equal(results.innerHTML, '', 'nothing was appended') + assert.notOk(remoteInput.hasAttribute('loading'), 'loading attribute was removed') done() }) input.value = 'test' input.focus() + assert.ok(remoteInput.hasAttribute('loading'), 'loading attribute was added') }) it('repects param attribute', function(done) { From 14e86c04aab99268a7bd711f6f902f6e1dd5e860 Mon Sep 17 00:00:00 2001 From: Mu-An Chiou Date: Tue, 10 Sep 2019 15:06:09 -0400 Subject: [PATCH 6/6] Document new custom events --- README.md | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index b47c08f..1076c37 100644 --- a/README.md +++ b/README.md @@ -45,23 +45,12 @@ remote-input[loading] .loading-icon { display: inline; } ### Events -```js -const remoteInput = document.querySelector('remote-input') - -// Network request lifecycle events. -remoteInput.addEventListener('loadstart', function(event) { - console.log('Network request started', event) -}) -remoteInput.addEventListener('loadend', function(event) { - console.log('Network request complete', event) -}) -remoteInput.addEventListener('load', function(event) { - console.log('Network request succeeded', event) -}) -remoteInput.addEventListener('error', function(event) { - console.log('Network request failed', event) -}) -``` +- `loadstart` - The server fetch has started. +- `load` - The network request completed successfully. +- `error` - The network request failed. +- `loadend` - The network request has completed. +- `remote-input-success` – Received a successful response (status code 200-299). Bubbles. +- `remote-input-error` – Received a not successful response. Bubbles. ## Browser support