p-map icon indicating copy to clipboard operation
p-map copied to clipboard

Allow breaking from iterator

Open papb opened this issue 4 years ago • 15 comments

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?

papb avatar Oct 13 '20 14:10 papb

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)

sindresorhus avatar Oct 13 '20 16:10 sindresorhus

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]...?

papb avatar Oct 13 '20 17:10 papb

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?

sindresorhus avatar Oct 13 '20 20:10 sindresorhus

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 avatar Oct 13 '20 22:10 papb

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)

papb avatar Oct 13 '20 22:10 papb

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

sindresorhus avatar Oct 13 '20 23:10 sindresorhus

: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

papb avatar Oct 14 '20 19:10 papb

Good idea 👍

sindresorhus avatar Oct 14 '20 23:10 sindresorhus

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

Richienb avatar Oct 29 '20 03:10 Richienb

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

papb avatar Oct 29 '20 04:10 papb

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

sindresorhus avatar Oct 29 '20 12:10 sindresorhus

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

sindresorhus avatar Jun 02 '21 08:06 sindresorhus

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

sindresorhus avatar Dec 10 '22 20:12 sindresorhus

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 avatar Jul 16 '23 11:07 Richienb

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.

Richienb avatar Jul 16 '23 11:07 Richienb