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

Rule proposal: Prefer Array.from() callback for list comprehensions #2550

Open
geoffswift opened this issue Feb 2, 2025 · 3 comments
Open

Comments

@geoffswift
Copy link

geoffswift commented Feb 2, 2025

Description

I've seen a bunch of examples where Array(n).keys() is used as part of a list comprehension. Since keys() here returns an iterator the typical form is like Array.from(Array(n).keys()) or [...Array(n).keys()]

This has the overhead of extra intermediate array(s) and using the callback for Array.from() although more verbose is the best performer.

See examples for details.

Examples

// ❌
Array.from(Array(n).keys())

// ❌
[...Array(n).keys()]

// ✅
Array.from({ length: n }, (_, i) => i)
// ❌
Array.from(Array(n).keys()).map((i) => ({ id: i, }));

// ❌
[...Array(n).keys()].map((i) => ({ id: i, }));

// ✅
Array.from({ length: n }, (_, i) => ({ id: i, }))

Proposed rule name

prefer-callback-for-list-comprehension

Additional Info

No response

@fregante
Copy link
Collaborator

fregante commented Feb 5, 2025

  1. Array(n) is already turned into new Array(n) by https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/new-for-builtins.md
  2. which is then turned into Array.from({length: n}) by https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-new-array.md

What happens with these two rules when they're run on those ❌ examples?

I'm not sure I like Array.from({ length: n }, (_, i) => i) though, it's less readable than Array.from({ length: n }).keys() even if more efficient, probably

@geoffswift
Copy link
Author

Thanks for clarifying the rule interactions. This is the current state of affairs with those rules enabled...

// ❌
Array.from(Array(n).keys())
// ✅
Array.from(Array.from({ length: n }).keys())

// ❌
[...Array(n).keys()]
// ✅
[...Array.from({ length: n }).keys()]

// ❌
Array.from(Array(n).keys()).map((i) => ({ id: i, }));
// ✅
Array.from(Array.from({ length: n }).keys()).map((i) => ({ id: i }))

// ❌
[...Array(n).keys()].map((i) => ({ id: i, }));
// ✅
[...Array.from({ length: n }).keys()].map((i) => ({ id: i }));

I am inclined to agree about readability for Array.from({ length: n }, (_, i) => i), when you're just generating a list of numbers. That said, if you are working on a project for which you value performance, you might be happy to turn the rule on and sacrifice some readability.

My examples with the map call are a bit more compelling to be fair. If we can move the callback from map inside Array.from that would seem tidy.

@omril1
Copy link

omril1 commented Feb 7, 2025

Although different example, but maybe #2357 can cover both use cases, can be named something like prefer-array-from-mapper

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

No branches or pull requests

3 participants