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

Add TypeScript definition #2

Merged
merged 2 commits into from Apr 12, 2019
Merged

Add TypeScript definition #2

merged 2 commits into from Apr 12, 2019

Conversation

BendingBender
Copy link
Contributor

No description provided.

@sindresorhus
Copy link
Owner

Accept Iterables as input

I accepted this in https://github.com/sindresorhus/random-item, but in hindsight, I probably shouldn't have. There are downsides with accepting Iterables. Strings are iterables, so users could accidentally specify a string, which would almost never be intended. It's also very easy to spread an Iterable at the callsite. My opinion is that methods should only accept an Iterable when it's actually beneficial, meaning, the function can work with the iterator directly instead of just converting it to an array right away. An example of this is: https://github.com/sindresorhus/p-reduce/blob/master/index.js

Any thoughts about this?

@BendingBender
Copy link
Contributor Author

Yeah, you're right, it's really very easy to just spread an iterable. I'll revert the changes here. Do you want me to revert them in random-item, too?

@sindresorhus
Copy link
Owner

Do you want me to revert them in random-item, too?

Yes :)

@sindresorhus sindresorhus changed the title Accept Iterables as input, add TypeScript definition Add TypeScript definition Apr 12, 2019
@sindresorhus sindresorhus merged commit 35c640c into sindresorhus:master Apr 12, 2019
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

Successfully merging this pull request may close these issues.

None yet

2 participants