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

Remove deprecated rules #1881

Merged
merged 10 commits into from
May 12, 2022
Merged

Remove deprecated rules #1881

merged 10 commits into from
May 12, 2022

Conversation

FloEdelmann
Copy link
Member

@FloEdelmann FloEdelmann commented May 7, 2022

Previously deprecated rules were removed completely:

@FloEdelmann FloEdelmann marked this pull request as draft May 7, 2022 12:47
@FloEdelmann
Copy link
Member Author

The test fails because vue/experimental-script-setup-vars is still used in tests/lib/script-setup.js. See #1882.

@ota-meshi
Copy link
Member

I wanted to discuss with you about removing the rule.
ESLint core doesn't seem to remove deprecation rules.
https://eslint.org/docs/user-guide/rule-deprecation
Do you think we should adopt this policy too?

@FloEdelmann
Copy link
Member Author

FloEdelmann commented May 9, 2022

Hmm, I tend to disagree here. Not removing deprecated rules might sound like it's making user's lives easier (since they don't have to make any changes when upgrading to the next major release), but I'd argue it's actually making it harder for users and maintainers:

  • First, users have to realize that a rule is actually unmaintained. If it's completely gone instead, ESLint will throw an error and users will have to look at the release notes and docs to find out what to do now.
  • If they encounter a bug with a deprecated rule, it's likely already fixed in the replacement rule. If users don't notice that themselves, they'll open an issue, which causes more work on both sides than just switching to the new rule.
  • What does "no maintenance" mean exactly? When we introduce a new ESLint rule in our codebase (e.g. some time ago I introduced eslint-plugin-unicorn), shouldn't we fix these new lint errors in the whole codebase? I.e. also in deprecated/unmaintained rules?

Also, there will always be other breaking changes that require user action anyway, like changing a default option or dropping support for older Node versions etc.

If users don't want breaking changes, they can still choose to not upgrade to new major versions. Then they will make the tradeoff "no maintenance for all rules" vs. "the configuration works as is" for themselves.

That's why I think that deprecating a rule in a minor release and removing it in the next major release is a viable and fair strategy that saves time and work.

@FloEdelmann FloEdelmann marked this pull request as ready for review May 9, 2022 11:33
@FloEdelmann FloEdelmann requested a review from ota-meshi May 10, 2022 15:04
@ota-meshi
Copy link
Member

Thank you for your opinion! I agree with you. Let us remove the deprecated rules.

I think it's better to leave a document about the remove rules. I think maybe we need to move the list in the ## Deprecated section somewhere.

@FloEdelmann
Copy link
Member Author

@ota-meshi I have added the rules docs back, and made clear in the docs that the rules are removed.

Please see https://deploy-preview-1881--eslint-plugin-vue.netlify.app/rules/#removed

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you very much

…eprecated

# Conflicts:
#	docs/rules/README.md
@ota-meshi ota-meshi merged commit a0cf018 into master May 12, 2022
@ota-meshi ota-meshi deleted the remove-deprecated branch May 12, 2022 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants