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

📎 Implement useArrayLiterals - eslint/no-array-constructor #2532

Closed
unvalley opened this issue Apr 19, 2024 · 9 comments
Closed

📎 Implement useArrayLiterals - eslint/no-array-constructor #2532

unvalley opened this issue Apr 19, 2024 · 9 comments
Assignees
Labels
A-Linter Area: linter good first issue Good for newcomers L-JavaScript Language: JavaScript and super languages

Comments

@unvalley
Copy link
Member

unvalley commented Apr 19, 2024

Description

no-array-constructor - ESLint - Pluggable JavaScript Linter

Want to contribute? Lets you know you are interested! We will assign you to the issue to prevent several people to work on the same issue. Don't worry, we can unassign you later if you are no longer interested in the issue! Read our contributing guide and analyzer contributing guide.

Name suggestions are welcome.

@unvalley unvalley added good first issue Good for newcomers A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Apr 19, 2024
@Kazuhiro-Mimaki
Copy link
Contributor

I'm interested in but I have never contributed biome rules, nor oss by rust.
Can I try this issue?

@unvalley
Copy link
Member Author

@Kazuhiro-Mimaki Hi, yes we can help! Firstly, I recommend reading CONTRIBUTING.md and other PRs implement lint rules.

@Conaclos
Copy link
Member

Conaclos commented Apr 21, 2024

I wonder if the rule should also ban new Array(length), as this creates an array with holes (which is bad for performance), and is only marginally useful for pre-initializing very large arrays (65 000 elements and more - and even in that case, I think using a literal array is fine).

If we accept new Array(length), we should use a better name such as noNewArrayInitiialization.

@unvalley
Copy link
Member Author

If we trust the results of this benchmark, it appears that insertion of an element into an array with holes is not that different from using a literal array.

The new Array(lengh) looks slightly worse in performance, but I am not sure if the lint rule should prohibit it.

@minht11
Copy link
Contributor

minht11 commented Apr 22, 2024

If user wants to create explicit size array, they could use

const length = 10;
const array = Array.from({length})

It is more explicit and performance difference should be nonexistant compared Array(length)
See also unicorn/no-new-array

@Conaclos
Copy link
Member

Conaclos commented Apr 22, 2024

If user wants to create explicit size array, they could use

const length = 10;
const array = Array.from({length})

Not sure users will appreciate the workaround.

Ok, let's allow new Array(length) for now. We could change this later, or add a new rule to ban it.

@unvalley and @minht11 what do you think about renaming the rule to useLiteralArray?

@minht11
Copy link
Contributor

minht11 commented Apr 22, 2024

I see that biome already has quite a few rules with word literal in them. So it sounds good to me from consistency and semantics standpoint.

Personally I would flip it to useArrayLiteral because it sounds more natural to me, but thats minor.

@unvalley
Copy link
Member Author

unvalley commented Apr 22, 2024

We have following named rules (related?):

useArrayLiterals?

@Conaclos
Copy link
Member

It seems that we have a winner: useArrayLiterals :)

@unvalley unvalley changed the title 📎 Implement noArrayConstructor - eslint/no-array-constructor 📎 Implement useArrayLiterals - eslint/no-array-constructor Apr 22, 2024
Kazuhiro-Mimaki added a commit to Kazuhiro-Mimaki/biome that referenced this issue Apr 23, 2024
Kazuhiro-Mimaki added a commit to Kazuhiro-Mimaki/biome that referenced this issue Apr 23, 2024
Kazuhiro-Mimaki added a commit to Kazuhiro-Mimaki/biome that referenced this issue Apr 24, 2024
Kazuhiro-Mimaki added a commit to Kazuhiro-Mimaki/biome that referenced this issue Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter good first issue Good for newcomers L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

No branches or pull requests

4 participants