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

Match [].some() behavior #17

Closed
fregante opened this issue Sep 9, 2023 · 10 comments
Closed

Match [].some() behavior #17

fregante opened this issue Sep 9, 2023 · 10 comments

Comments

@fregante
Copy link

fregante commented Sep 9, 2023

I think I see two main differences:

  • count is required, while it should likely just default to 1
  • [].some(() => true) just returns false while pSome([], {count:1}) will throw
  • p-some is acting as a limited filter rather than a boolean-returning function

I'd suggest moving this limiting behavior to https://github.com/sindresorhus/p-filter or rethink this this pattern with the upcoming iterator helpers: https://github.com/tc39/proposal-iterator-helpers#takelimit

For example: [...promises].filter(filtererFn).take(2)

@fregante
Copy link
Author

fregante commented Sep 9, 2023

Note: When count === 1 and filter === undefined, the current behavior matches Promise.any exactly.

@fregante
Copy link
Author

fregante commented Sep 9, 2023

This is my local implementation of a boolean p-some for Refined GitHub:

export default async function pSome(iterable: Iterable<Promise<unknown>>): Promise<boolean> {
	return new Promise(resolve => {
		for (const promise of iterable) {
			(async () => {
				if (await promise) {
					resolve(true);
				}
			})();
		}

		void Promise.allSettled(iterable).then(() => {
			resolve(false);
		});
	});
}

@sindresorhus
Copy link
Owner

While the naming is unfortunately because of Array#some, is it really worth doing a big breaking change at this point? The package is stable and it will eventually be replaced by built-in methods.

Maybe do a named export for this behavior or a separate package?

@fregante
Copy link
Author

fregante commented Sep 9, 2023

at this point?

Before this point: 7 years
After this point: eternity

Never too late 😃

big breaking change

As long as there's an easy upgrade path I don't see the problem with it.

it will eventually be replaced by built-in methods

Are you talking about https://github.com/tc39/proposal-iterator-helpers#somefn

You may be right, it might be possible to use Iterator.from(array).some(fn)

@sindresorhus
Copy link
Owner

Before this point: 7 years

Counterpoint: No one needed what you propose in 7 years of its existence.

@sindresorhus
Copy link
Owner

As long as there's an easy upgrade path I don't see the problem with it.

What is the upgrade path? If we change from returning the values to returning a boolean, I don't really see how the user could even upgrade.

@fregante
Copy link
Author

fregante commented Sep 9, 2023

Counterpoint: No one needed what you propose in 7 years of its existence.

It's more likely that they settled for its booleanish equivalence and there are plenty of bugs caused by Promise<false> in the wild. Given that .some() is part of the tiny standard lib of JavaScript I'd dare to say more than "no one" needs this specific behavior.

@sindresorhus
Copy link
Owner

I only use it for the current behavior.

@sindresorhus
Copy link
Owner

Totally irrelevant, but I find the name Array#some confusing. In a proper language, it would have simply been an overload of Array#includes. It should at minimum have been named Array#hasSome. This package name is modeled after Promise.all => Promise.some. If you think about it like that, the current behavior makes more sense, it returns some elements, not all.

@fregante
Copy link
Author

fregante commented Sep 9, 2023

As for the modeling, it looks this matches Bluebird's .some() method exactly so I'll close this issue.

http://bluebirdjs.com/docs/api/promise.some.html

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants