Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow breaking from iterator #31

Open
papb opened this issue Oct 13, 2020 · 15 comments
Open

Allow breaking from iterator #31

papb opened this issue Oct 13, 2020 · 15 comments

Comments

@papb
Copy link
Contributor

papb commented Oct 13, 2020

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 sindresorhus/p-each-series#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
Copy link
Owner

sindresorhus commented Oct 13, 2020

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
Copy link
Contributor Author

papb commented Oct 13, 2020

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
Copy link
Owner

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
Copy link
Contributor Author

papb commented Oct 13, 2020

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 😅

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
Copy link
Contributor Author

papb commented Oct 13, 2020

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
Copy link
Owner

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

@papb
Copy link
Contributor Author

papb commented Oct 14, 2020

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

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
Copy link
Owner

Good idea 👍

@Richienb
Copy link
Contributor

Richienb commented Oct 29, 2020

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

@papb
Copy link
Contributor Author

papb commented Oct 29, 2020

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

@sindresorhus
Copy link
Owner

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

@sindresorhus
Copy link
Owner

Also see the discussion in sindresorhus/p-times#1.

@sindresorhus
Copy link
Owner

If anyone wants to work on this, see the initial attempt in #36

@Richienb
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants