5 ms·

This code does not recapitulate the desired behavior. Desired: when the load from chapter 10 fails, even if chapter 2 is taking 30s to load, I want to immediat

This code does not recapitulate the desired behavior. Desired: when the load from chapter 10 fails, even if chapter 2 is taking 30s to load, I want to immediately transition the UI into the failure state. Your code: waits until chapters 1-9 have loaded to report the failure state on chapter 10. (I posted I believe a correct answer "without the hack" to the OP.)

scotty79 4y ago
How can you create that behavior?
- crdrost 4y ago
  Like I said I added a solution in the Disqus thread to that blog post... the English summary is that you Promise.race() the Promise.all() with the for-await loop.
  - remram 4y ago
    Does that work? Isn't it possible for the monitor() function to return the result of Promise.all() rather than loop(), since they are racing? return Promise.race(Promise.all(promises), loop()); edit: I guess they are the same in this specific case, but probably wouldn't be in real code, making this function very clever but maybe too rigid.
    - scotty79 4y ago
      I think that it works because when loop() rejects, Promise.all() rejects too so it doesn't matter who wins.
      - remram 4y ago
        I mean in the case where nothing rejects. Say for example the loop was doing `result.push({success: true, value: item})` instead of just `result.push(item)`. Then the value returned from loop() is different from the one returned by Promise.all(promises), and your overall monitor() function might return either.
spankalee 4y ago
I'm not trying to say there's only one way to do this, especially if we are talking about user affordances for failure states. I think there could be a debate about showing what data is available vs not showing partial data. My point is just that trying to do something with the failures often removes the warning and leads to better UX, or at least conscious decisions about it, and usually some clue in the code about what you intend to happen.
wruza 4y ago
Does the article’s final code behave in a desired way? To my understanding, it simply catches rejections, but for-await block still waits for every promise in order, up until the rejected one. Your `finally` block gets called too late, unless I miss something obvious. To make this loop early-cancellable on any out-of-order rejection, I’d create a helper generator and a “semaphore” promise, which would block the yield until there’s a next resolved value or any rejection. const arr = [] arr.length = promises.length let step, sem function rearm() { sem = new Promise(res => {step = res}) } rearm() promises.forEach((p, i) => { p.then(v => step({i, v})) .catch(e => step({i, e})) }) async function* values() { let next = 0 while (next < arr.length) { if (arr[next]) { yield arr[next++].v continue } const {i, v, e} = await sem rearm() if (e) { yield Promise.reject(e) return } arr[i] = {v} } } try { for await (… of values()) { … I barely ever used js generators specifically, so this code is more like a concept (e.g. I suspect a race around rearm()), but you get the idea.
lhorie 4y ago
Not sure what `Promise.race(Promise.all(promises), loop());` is supposed to be doing. Promise.race only takes one arg, no? Also not sure what partialUpdate is supposed to be doing. Is that assuming some React-like diff reconciliation? Personally, I'd go with something like this: async function showChapters(chapterURLs) { return Promise.all( chapterURLs .map((url) => fetch(url).then(r => r.json())) .map((p, i) => p.then(data => appendChapter(data, i), e => displayError(e, i))) ); } With this, you can control exactly where and how list items are rendered, regardless of order of promise resolution, and things are rendered as soon as they are available, instead of getting stuck behind slow requests.

scotty79 4y ago

How can you create that behavior?

crdrost 4y ago
Like I said I added a solution in the Disqus thread to that blog post... the English summary is that you Promise.race() the Promise.all() with the for-await loop.
- remram 4y ago
  Does that work? Isn't it possible for the monitor() function to return the result of Promise.all() rather than loop(), since they are racing? return Promise.race(Promise.all(promises), loop()); edit: I guess they are the same in this specific case, but probably wouldn't be in real code, making this function very clever but maybe too rigid.
  - scotty79 4y ago
    I think that it works because when loop() rejects, Promise.all() rejects too so it doesn't matter who wins.
    - remram 4y ago
      I mean in the case where nothing rejects. Say for example the loop was doing `result.push({success: true, value: item})` instead of just `result.push(item)`. Then the value returned from loop() is different from the one returned by Promise.all(promises), and your overall monitor() function might return either.

spankalee 4y ago

I'm not trying to say there's only one way to do this, especially if we are talking about user affordances for failure states. I think there could be a debate about showing what data is available vs not showing partial data. My point is just that trying to do something with the failures often removes the warning and leads to better UX, or at least conscious decisions about it, and usually some clue in the code about what you intend to happen.

wruza 4y ago

Does the article’s final code behave in a desired way? To my understanding, it simply catches rejections, but for-await block still waits for every promise in order, up until the rejected one. Your `finally` block gets called too late, unless I miss something obvious. To make this loop early-cancellable on any out-of-order rejection, I’d create a helper generator and a “semaphore” promise, which would block the yield until there’s a next resolved value or any rejection. const arr = [] arr.length = promises.length let step, sem function rearm() { sem = new Promise(res => {step = res}) } rearm() promises.forEach((p, i) => { p.then(v => step({i, v})) .catch(e => step({i, e})) }) async function* values() { let next = 0 while (next < arr.length) { if (arr[next]) { yield arr[next++].v continue } const {i, v, e} = await sem rearm() if (e) { yield Promise.reject(e) return } arr[i] = {v} } } try { for await (… of values()) { … I barely ever used js generators specifically, so this code is more like a concept (e.g. I suspect a race around rearm()), but you get the idea.

lhorie 4y ago

Not sure what `Promise.race(Promise.all(promises), loop());` is supposed to be doing. Promise.race only takes one arg, no? Also not sure what partialUpdate is supposed to be doing. Is that assuming some React-like diff reconciliation? Personally, I'd go with something like this: async function showChapters(chapterURLs) { return Promise.all( chapterURLs .map((url) => fetch(url).then(r => r.json())) .map((p, i) => p.then(data => appendChapter(data, i), e => displayError(e, i))) ); } With this, you can control exactly where and how list items are rendered, regardless of order of promise resolution, and things are rendered as soon as they are available, instead of getting stuck behind slow requests.