sindresorhus / p-map

Map over promises concurrently
MIT License
1.27k stars 58 forks source link

Allow breaking from iterator #31

Open papb opened 3 years ago

papb commented 3 years ago

I wish I could stop the iteration early by returning something like pMap.stop (a Symbol) from the iterator.

This is basically the same request as https://github.com/sindresorhus/p-each-series/issues/3

For now, I have to throw an error on purpose just to break it. Thankfully, I don't need the return value of p-map anyway.

But with this feature, the parts of the array that weren't computed yet could be set to undefined, or pMap.uncomputed (another Symbol), or even be specified by a valueForUncomputedEarlyStop option.

Can I do a PR?

sindresorhus commented 3 years ago

Couldn't we just document that the returned array will be incomplete if the user stops early?

Yes, PR welcome. pMap.stop sounds good.

You can find inspiration for the TS types in https://github.com/sindresorhus/find-up/blob/master/index.d.ts (and testing and docs too, probably)

papb commented 3 years ago

Couldn't we just document that the returned array will be incomplete if the user stops early?

But this sentence alone leaves room for uncertainty on what happens with the intermediate missing values, for example if we are mapping over [a, b, c] and stop early before b is done (with a and c already done), would you expect [a, undefined, c], [a, <empty>, c], [a, c]...?

sindresorhus commented 3 years ago

There are two use-cases I can think of:

  1. Mapping and not caring about the indexes, which means the returned array could be Array#flat()'ed.
  2. Mapping and needing the resulting array to have the same indices, even if incomplete.

So either we can:

  1. Add a pMap option for which behavior to use.
  2. Add two .stop properties where which you choose decides the behavior.
  3. Just return at original indices, and if the user doesn't care about the indices they can Array#flat() themselves.

Any opinions? What would go with?

papb commented 3 years ago

At first thought, I would prefer 3, but for that we would have to return [a,,c] instead of [a,undefined,c], because Array#flat() only works in the former. That would also be a way of differentiating between "incomplete" and "complete returning undefined".

However, sparse arrays are usually frowned upon. For example, I remember reading somewhere that TypeScript made a design choice of pretending that sparse arrays don't exist, in order to vastly simplify something.

That said, I prefer option 1 now, filling with undefineds when the user chooses the 'keep indexes' option. And if someone also needs to differentiate between undefined and incomplete, then they will have to make their own logic :sweat_smile:

Option 2 is also interesting, but I suspect that most users will not need the returned array at all if they're breaking early, so what it returns will be irrelevant most of the time, so I think having a single, short-named pMap.stop would be better.

papb commented 3 years ago

In short: I would go with a single pMap.stop symbol and a sparsedEarlyStop option which defaults to true (filling with undefined on the incomplete indexes). If set to false, returns a shorter array instead. (feel free to suggest another name for the option, of course)

sindresorhus commented 3 years ago

I agree with your assessment. Let's go with that. I think we need a better name for that option though.

papb commented 3 years ago

:thinking: What happens with the index in which the early stop happened? Does it become undefined as well? :thinking:

I had a new idea. Instead of providing a simple pMap.stop symbol, what if we provide a pMap.stop function, and use like this?

await pMap(someArray, async element => {
  // ...
  return pMap.stop({
    value: 123,
    missingIndexes: {
      collapse: false,
      fillWith: 'i-was-not-computed'
    }
  });
});
//=> ['foo', 'i-was-not-computed', 'bar', 123, 'i-was-not-computed', 'baz']
//                                        ^^^ this element caused the early stop
sindresorhus commented 3 years ago

Good idea 👍

Richienb commented 3 years ago

Do we want to call .cancel or similar on all of the incomplete promises if it is stopped?

papb commented 3 years ago

Nice idea @Richienb! We could detect if each promise supports canceling and cancel them.

sindresorhus commented 3 years ago

Ideally, we would also support AbortController, but that can be done later. Just something to keep in mind.

sindresorhus commented 3 years ago

Also see the discussion in https://github.com/sindresorhus/p-times/issues/1.

sindresorhus commented 1 year ago

If anyone wants to work on this, see the initial attempt in https://github.com/sindresorhus/p-map/pull/36

Richienb commented 1 year ago

Idea: pMap.stop could accept an option that prevents values further down the array from being evaluated but ensures values before it are. This would make the array completely full. Then, we return all values before the one that returned pMap.stop.

Richienb commented 1 year ago

Another idea: one use case I can think of for early stopping is an async version of Array#find. In that case, an API that returns both the item that caused the stop and then the other values could be useful.