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

[eslint-plugin] feat: split no-deprecated-components rule #5478

Merged
merged 7 commits into from
Aug 4, 2022

Conversation

adidahiya
Copy link
Contributor

This PR creates new ESLint rules which flag usage of deprecated components specific to each @blueprintjs package:

  • @blueprintjs/no-deprecated-core-components
  • @blueprintjs/no-deprecated-datetime-components
  • @blueprintjs/no-deprecated-select-components
  • @blueprintjs/no-deprecated-timezone-components

Rationale (also in README): In migrations of large code bases, it may be useful to apply more granular rule configuration of "no-deprecated-components" to make incremental progress towards the newer APIs. This allows you, for example, to flag deprecated @blueprintjs/core component usage as ❌ errors while allowing deprecated components from other packages
to pass as lint ⚠️ warnings.

Note that @blueprintjs/no-deprecated-components has not been removed, it is still available, and is enabled in the "@blueprintjs/recommended" config (this config is not used in our internal monorepos, btw, just provided for convenience and seems to be an ESLint plugin convention). I refactored the implementation of this rule into a reusable function so that all 5 rules share the same implementation.

Following up from #5477 (comment) and other conversations... the other idea I had of introducing a configuration object for @blueprintjs/no-deprecated-components to set "error"/"warn" levels for the different Blueprint packages turned out to be messy and less legible than the approach taken in this PR. It's better to let ESLint's infrastructure handle the "error"/"warn" option toggling, and splitting into separate rules also helps with legibility because we can search the codebase for // eslint-disable-next-line @blueprintjs/no-deprecated-core-components lint flags.

@adidahiya adidahiya requested a review from styu August 3, 2022 18:37
@blueprint-bot
Copy link

Fix lint with disable flags

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

fix timezone lint with disable flags

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

address CR, DRY code

Previews: documentation | landing | table | demo

@blueprint-bot
Copy link

fix more lint

Previews: documentation | landing | table | demo

@adidahiya
Copy link
Contributor Author

@styu this is ready for another round of review when you get the chance

* package instead.
*/

/* eslint-disable deprecation/deprecation, @blueprintjs/no-deprecated-components */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize it is redundant to have both of these rules on as "error" violations, but I'm keeping them both so we can dogfood our own lint rule.

@adidahiya adidahiya merged commit b0611b5 into develop Aug 4, 2022
@adidahiya adidahiya deleted the ad/split-no-deprecated-components-rule branch August 4, 2022 16:50
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

3 participants