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

Reconfigure standard and eslint #1668

Merged

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Jan 2, 2022

Pull Request

Problem

We were installing standard, but then running eslint and trying to access its dependency configuration files rather than depending on their packages explicitly. This is fragile.

standard cli not usable because of custom eslint rules.

The TypeScript config was not using standard base configuration due to previous annoyance with the multiple dependencies.

Solution

Install standard ESLint configurations and not the whole package. Install necessary peer dependencies, so works with npm below npm 8 which require explicitly installing peer dependencies. Basically same dependencies for eslint-config-standard and eslint-config-standard-with-typescript so go back to using standard for TypeScript too.

Used eslint --fix to fix most of the problems picked up. Nice, don't need standard CLI to get --fix!

The downside is we still have to juggle dependencies when some standard dependencies lag behind versions of ESLint or TypeScript, like currently still waiting for one last package (eslint-plugin-node) to get ESLint 8 support.

Considered another approach but rejected: switch to using semistandard and ts-standard instead of eslint. Reasons:

  • we prefer semicolons, and while there is semistandard there is no ts-semistandard
  • there is VSCode extension for standard, but not sure if can work with both semistandard and ts-standard
  • we have some custom rules, and the point of standard is no configuration
  • (I like ESLint, and ESLint plugin to VSCode, and being able to add custom rules!)

ChangeLog

  • refactor standard related devDependencies and usage

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented Jan 2, 2022

Reminder to self on possible dependency update approach with less trial and error.

  1. standard related: check peerDependencies of eslint-config-standard and eslint-config-standard-with-typescript for supported ranges. e.g. as of 2022-01-03 requires ESLint 7.
  "peerDependencies": {
    "eslint": "^7.12.1",
    "eslint-plugin-import": "^2.22.1",
    "eslint-plugin-node": "^11.1.0",
    "eslint-plugin-promise": "^4.2.1 || ^5.0.0"
  },

(npm 8 auto-installs peer dependencies so hopefully make these implicit in future, but currently adding these dependencies explicitly to Commander so developer install still works with npm 6.)

  1. TypeScript

Check typescript-eslint for supported TypeScript version.

Reminder: need to use tsd with same embedded version of TypeScript to avoid surprises, as tsd includes a custom build which currently overwrites node_modules/.bin/tsc link.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

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

great job!

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jan 4, 2022
@shadowspawn shadowspawn added this to the Commander v9.0.0 milestone Jan 4, 2022
@shadowspawn shadowspawn merged commit 10b673f into tj:release/9.x Jan 4, 2022
@shadowspawn shadowspawn deleted the feature/use-standard-config-advanced branch January 4, 2022 05:14
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jan 29, 2022
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.

2 participants