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

Handle deprecated rules (closes #112) #120

Closed

Conversation

alexilyaev
Copy link
Contributor

Deprecated rules should be added conditionally.

See #112

@alexilyaev
Copy link
Contributor Author

alexilyaev commented Oct 18, 2019

@lydell Let's continue the discussion here.

I've gone with the simplest solution...

  • Get the version of a dependency from user's package.json (relative to process.cwd())
  • Add the deprecated rules conditionally based on a semver version comparison
  • Gracefully fail if not found, as if the found version is 0.0.0 (i.e. no breaking changes)

Notes

  • We can assume process.cwd() is the right one, since eslint-find-rules assumes that as well.
  • Worst case we just don't find the dependency versions and the deprecated rules are added as they are today.

@lydell
Copy link
Member

lydell commented Oct 19, 2019

Let’s take indent-legacy as an example. It was deprecated in some ESLint version X. But it’s still around. People might take their time to upgrade to the new indent rule, but still update ESLint. Then they start adding Prettier to their project, piece by piece. Then wouldn’t they want eslint-config-prettier to disable indent-legacy for them?

index.js Outdated Show resolved Hide resolved
@alexilyaev
Copy link
Contributor Author

alexilyaev commented Oct 19, 2019

Let’s take indent-legacy as an example. It was deprecated in some ESLint version X. But it’s still around. People might take their time to upgrade to the new indent rule, but still update ESLint. Then they start adding Prettier to their project, piece by piece. Then wouldn’t they want eslint-config-prettier to disable indent-legacy for them?

Ok, looks like I misunderstood the deprecation process. I thought that deprecated rules are removed and don't work when you upgrade, but it turns out they stick around and will work indefinitely, as mentioned in the ESLint Rule Deprecation guidelines.

So now let me rethink this.
Let's try another example...

  • Using an old ESLint version (e.g. v3.0.0)
  • Adding the rule no-spaced-func
  • Upgrading to latest ESLint, keeping the rule in .eslintrc
  • Adding Prettier
  • Formatting files with Prettier
  • Linting with ESLint

In this case, there's actually no conflict as Prettier is aligned with the expected behavior of no-spaced-func, but assuming Prettier could theoretically change the behavior (as they did with Arrow Functions parameters braces when there's only one parameter), then it could actually conflict and ESLint would yell at the user.

This is where adding eslint-config-prettier would be expected to solve the problem, by turning no-spaced-func off.

This means that we can't rely on the ESLint version as users may still use the deprecated rules.

Let me define my original challenge...

  • I'd like to keep my ESLint configs up to date, without any deprecated rule, as those are not maintained and may not work properly for future code.
  • I'd like to have an automated process to fail a project lint process or CI process when deprecated rules are found
  • ESLint doesn't warn when using deprecated rules, it only adds a list of usedDeprecatedRules in the Node API results, which is accessible only after running ESLint on some files (Fix: Warn for deprecation in Node output (fixes #7443) eslint/eslint#10953)
  • So I'm using eslint-find-rules to error on used deprecated rules
  • This works, but when using eslint-config-prettier, eslint-find-rules errors since we turn off some rules that are deprecated

Thoughts

Two options come to mind, based on your suggestions:

  1. Support an environment variable, if exists, don't add the deprecated rules, e.g.
ESLINT_CONFIG_PRETTIER_NO_DEPRECATED=1 npx eslint-find-rules -d .eslintrc
  1. Support a mirrored config that doesn't include deprecated rules
    • prettier/no-legacy
    • prettier/react-no-legacy

I'm leaning towards 1 since it's much simpler and fits well with the use case of running a CLI command to lint for deprecated rules.

I'll make a new PR with that solution.

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

Successfully merging this pull request may close these issues.

None yet

2 participants