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

prefer-spread: Add prefer spread over .slice()/.concat() #737

Closed
fisker opened this issue May 18, 2020 · 8 comments · Fixed by #1117
Closed

prefer-spread: Add prefer spread over .slice()/.concat() #737

fisker opened this issue May 18, 2020 · 8 comments · Fixed by #1117

Comments

@fisker
Copy link
Collaborator

fisker commented May 18, 2020

Currently, prefer-spread only check Array.from(), how about add prefer spread over .slice()/.concat()?

Fail

const foo = array.slice();
const foo = array.concat(item);
const foo = array.concat(anotherArray);

Pass

const foo = [...array];
const foo = array.slice(1);

The concat() case, we can't know item is array or not.

Maybe with suggestions?
Suggestion 1: [...array, itemOrArray]
Suggestion 2: [...array, ...itemOrArray]

Another problem is TypedArray#slice() and String#concat(). String#slice should not a problem. Nobody use string.slice()(no arguments).


Any other method people use to clone an array?

@sindresorhus
Copy link
Owner

The concat() case, we can't know item is array or not.

Maybe, if the user is using the TypeScript parser (#347), we could handle it?

And if not, just use suggestions.

@sindresorhus sindresorhus changed the title prefer-spread: Add prefer spread over .slice()/.concat() prefer-array-spread: Add prefer spread over .slice()/.concat() May 18, 2020
@sindresorhus
Copy link
Owner

Any other method people use to clone an array?

Array.from([1, 2]) and Array.of([1, 2]).

@fisker
Copy link
Collaborator Author

fisker commented May 18, 2020

@sindresorhus I mean add to our existing rule prefer-spread, not new rule.

@sindresorhus sindresorhus changed the title prefer-array-spread: Add prefer spread over .slice()/.concat() prefer-spread: Add prefer spread over .slice()/.concat() May 18, 2020
@fisker
Copy link
Collaborator Author

fisker commented Jan 26, 2021

I feel bad that I can't distinguish Array and TypedArray, so I can't make this rule work on Array#slice()

Let's close this?

@sindresorhus
Copy link
Owner

@fisker Maybe we can make it opt-in? I have personally never used or seen .slice() used on TypedArray. For me, it's more valuable to catch Array#slice(). // eslint-disable are good for edge-cases like this.

@coreyfarrell
Copy link
Contributor

Are you open to an option to ignore [].concat(value)? I use this pattern to normalize arguments where a single element or an array of elements is accepted. I know I could use Array.isArray(value) ? [...value] : [value] but IMO this is less readable.

@sindresorhus
Copy link
Owner

@coreyfarrell You can just use [value].flat() for that (we will eventually enforce that #975).

@coreyfarrell
Copy link
Contributor

That is a reasonable replacement, thanks!

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