10000 `prefer-ternary`: Check `if + return/throw` by fisker · Pull Request #1221 · sindresorhus/eslint-plugin-unicorn · GitHub
[go: up one dir, main page]
Skip to content

prefer-ternary: Check if + return/throw #1221

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

prefer-ternary: Check if + return/throw #1221

wants to merge 4 commits into from

Conversation

fisker
Copy link
Collaborator
@fisker fisker commented Apr 27, 2021

Fixes #1116

@fisker fisker force-pushed the prefer-ternary-improve branch from cb1a37f to 9bbfe66 Compare April 27, 2021 09:26
fisker added a commit to fisker/eslint-plugin-unicorn that referenced this pull request Apr 27, 2021
@fisker
Run sindresorhus#1221 on codebase
8875576
@fisker
Copy link
Collaborator Author
fisker commented Apr 27, 2021 •
edited
Loading

//cc @fregante @JounQin https://github.com/fisker/eslint-plugin-unicorn/pull/282/files I run this on our codebase, feel losing readability, look good to you?

@fregante
Copy link
Collaborator
fregante commented Apr 27, 2021 •
edited
Loading

Indeed there’s an existing issue with that rule:

  • prefer-ternary should probably not apply to long lines, it tends to make them less readable

a possible improvement would be to put the condition on its own constant.

Edit: #1079 (comment)

@JounQin
Copy link
Contributor
JounQin commented Apr 27, 2021

LGTM except @fregante mentioned.

@fisker
Copy link
Collaborator Author
fisker commented Apr 27, 2021 •
edited
Loading

This

if (a) {
	return 1;
}

if (b) {
	return 2;
}

if (c) {
	return 3;
} else {
	return 4;
}

seems fine to fix to

if (a) {
	return 1;
}

if (b) {
	return 2;
}

return c ? 3 : 4;

But

if (a) {
	return 1;
}

if (b) {
	return 2;
}

if (c) {
	return 3;
}

return 4;

seems bad to fix to

if (a) {}

if (b) {}

return c ? 3 : 4

@fisker
Copy link
Collaborator Author
fisker commented Apr 27, 2021 •
edited
Loading

Example this line https://github.com/fisker/eslint-plugin-unicorn/pull/282/files#diff-b6b472744fff513bca9c7577f37bc75b74371f8f7fab1621e4aae16ce15793dcL53, there are many other if above it.

@sindresorhus
Copy link
Owner

seems bad to fix to

Agreed

@fregante fregante mentioned this pull request Dec 11, 2021
Open
@sindresorhus
Copy link
Owner

Closing as this has been open for multiple years now without any activity.

@fisker fisker mentioned this pull request Aug 31, 2024
Closed
3 tasks
@fisker fisker mentioned this pull request Mar 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can prefer-ternary be merged into eslint core rule no-else-return?
4 participants
@fisker @fregante @JounQin @sindresorhus
0