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

Update major Node and Eslint versions #5509

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jorg-vr
Copy link
Contributor

@jorg-vr jorg-vr commented Apr 29, 2024

This pull request contains an update of eslint and a required update of node as well.

It replaces #5474 whcih gets frequently closed by dependabot. See dependabot/dependabot-core#7647

As this is a major version change, this also required some changes on our side:

  • I updated all github runners to node 20 as older versions are no longer supported
  • I moved our .eslintrc and .eslintignore to modern eslint.config.js using this migration guide (eslintrc is no deprecated)
  • I simplified the config by always using the typescript eslint config (Almost everything is typescript now anyway and the typescript specific rules dont seem to break on javascript files)
  • I fixed some minor issues the linter started complaining about
  • yarn lint now also explicitly lints the test directory

Before release we should

  • update our production servers to node 20 (tests should run in the same context as production)
  • set "no-unused-vars" rule back to error once this issue is fixed: markVariableAsUsed for the customElement decorator 43081j/eslint-plugin-lit#195
  • Discuss if we want to keep a line length warning (We now have hundreds of violations, which wouldn't be improved by shorthening them in my opninon)
  • Discuss if we want to enforce the 'no-explicit-any' rule. Because everything is typescript now, there is less need to use any.
  • Fix my editor to use the new config

dependabot bot and others added 8 commits April 8, 2024 11:47
Bumps the eslint group with 2 updates: [eslint](https://github.com/eslint/eslint) and [eslint-plugin-jest](https://github.com/jest-community/eslint-plugin-jest).


Updates `eslint` from 8.57.0 to 9.0.0
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md)
- [Commits](eslint/eslint@v8.57.0...v9.0.0)

Updates `eslint-plugin-jest` from 27.9.0 to 28.2.0
- [Release notes](https://github.com/jest-community/eslint-plugin-jest/releases)
- [Changelog](https://github.com/jest-community/eslint-plugin-jest/blob/main/CHANGELOG.md)
- [Commits](jest-community/eslint-plugin-jest@v27.9.0...v28.2.0)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: eslint
- dependency-name: eslint-plugin-jest
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: eslint
...

Signed-off-by: dependabot[bot] <support@github.com>
@jorg-vr jorg-vr added the dependencies Pull requests that update a dependency file label Apr 29, 2024
@jorg-vr jorg-vr self-assigned this Apr 29, 2024
@niknetniko
Copy link
Member

To give 2cents:

  • Discuss if we want to keep a line length warning (We now have hundreds of violations, which wouldn't be improved by shorthening them in my opninon)

I am strongly in favour of dropping the warning (I already do this locally).

  • Discuss if we want to enforce the 'no-explicit-any' rule. Because everything is typescript now, there is less need to use any.

I think this would be good, but it might be a lot of work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants