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

chore(deps-dev): bump to ESLint v9 #3081

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

chore(deps-dev): bump to ESLint v9 #3081

wants to merge 43 commits into from

Conversation

csouchet
Copy link
Member

@csouchet csouchet commented Apr 22, 2024

In ESLint v9, we must have only one configuration file.

Some dependencies don't support officially ESLint v9, but it works 😊


ℹ️ ESLint 8 has been EOL since October 5, 2024.
https://eslint.org/blog/2024/09/eslint-v8-eol-version-support/#eslint-v8.x-end-of-life-is-october-5%2C-2024


Need to support ESLint v9:

typescript-eslint V8: https://typescript-eslint.io/blog/announcing-typescript-eslint-v8

https://github.com/sindresorhus/eslint-plugin-unicorn (but supports flat config)

eslint-plugin-playwright: https://github.com/playwright-community/eslint-plugin-playwright?tab=readme-ov-file#usage

https://github.com/prettier/eslint-plugin-prettier: https://github.com/prettier/eslint-plugin-prettier?tab=readme-ov-file#configuration-new-eslintconfigjs

https://github.com/prettier/eslint-config-prettier: https://github.com/prettier/eslint-plugin-prettier?tab=readme-ov-file#configuration-new-eslintconfigjs

eslint-plugin-jest:

npm error Conflicting peer dependency: [email protected]  
npm error node\_modules/eslint  
npm error   peer eslint@"^8.56.0" from @typescript-eslint/[email protected]  
npm error   node\_modules/@typescript-eslint/eslint-plugin  
npm error     peerOptional @typescript-eslint/eslint-plugin@"^6.0.0 || ^7.0.0 || ^8.0.0" from [email protected]  
npm error     node\_modules/eslint-plugin-jest  
npm error       dev eslint-plugin-jest@"~28.8.3" from the root project  

eslint-plugin-jest-extended: https://github.com/jest-community/eslint-plugin-jest/blob/main/src/index.ts

✅ eslint-plugin-import: https://github.com/import-js/eslint-plugin-import?tab=readme-ov-file#config---flat-eslintconfigjs

eslint-import-resolver-typescript:

✅ eslint-plugin-notice: https://github.com/nickdeis/eslint-plugin-notice?tab=readme-ov-file#flag-config

@csouchet csouchet added the dependencies Pull requests that update a dependency (dev or runtime) label Apr 22, 2024
@csouchet csouchet force-pushed the chore/eslint9 branch 2 times, most recently from 43d040b to 32284cc Compare April 22, 2024 10:26
@csouchet csouchet changed the title chore(dev-deps): Bump to ESLint 9.1.0 chore(dev-deps): Bump to ESLint v9 May 24, 2024
@tbouffard tbouffard changed the title chore(dev-deps): Bump to ESLint v9 chore(dev-deps): bump to ESLint v9 May 24, 2024
@tbouffard tbouffard changed the title chore(dev-deps): bump to ESLint v9 chore(deps-dev): bump to ESLint v9 Aug 2, 2024
Copy link

github-actions bot commented Oct 8, 2024

🎊 PR Preview 63e555c has been successfully built and deployed to https://process-analytics-bpmn-visualization-js-doc_preview-pr-3081.surge.sh

🕐 Build time: 0.011s

🤖 By surge-preview

Copy link

github-actions bot commented Oct 8, 2024

🎊 PR Preview 63e555c has been successfully built and deployed to https://process-analytics-bpmn-visualization-js-demo_preview-pr-3081.surge.sh

🕐 Build time: 0.011s

🤖 By surge-preview

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@tbouffard tbouffard added the skip CI e2e tests GitHub Actions do not run e2e tests (for Pull Requests) label Oct 21, 2024
@csouchet csouchet force-pushed the chore/eslint9 branch 2 times, most recently from 91de37e to b61d045 Compare October 29, 2024 15:32
@csouchet csouchet force-pushed the chore/eslint9 branch 2 times, most recently from 41d62b4 to 4a843ec Compare November 18, 2024 15:22
dependabot bot and others added 10 commits January 7, 2025 09:56
Bumps the lint group with 6 updates:

| Package | From | To |
| --- | --- | --- |
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) | `7.4.0` | `7.6.0` |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) | `7.4.0` | `7.6.0` |
| [eslint](https://github.com/eslint/eslint) | `8.57.0` | `9.0.0` |
| [eslint-plugin-jest](https://github.com/jest-community/eslint-plugin-jest) | `27.9.0` | `28.2.0` |
| [eslint-plugin-playwright](https://github.com/playwright-community/eslint-plugin-playwright) | `1.5.4` | `1.6.0` |
| [eslint-plugin-unicorn](https://github.com/sindresorhus/eslint-plugin-unicorn) | `51.0.1` | `52.0.0` |

Updates `@typescript-eslint/eslint-plugin` from 7.4.0 to 7.6.0
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v7.6.0/packages/eslint-plugin)

Updates `@typescript-eslint/parser` from 7.4.0 to 7.6.0
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v7.6.0/packages/parser)

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)

Updates `eslint-plugin-playwright` from 1.5.4 to 1.6.0
- [Release notes](https://github.com/playwright-community/eslint-plugin-playwright/releases)
- [Changelog](https://github.com/playwright-community/eslint-plugin-playwright/blob/main/CHANGELOG.md)
- [Commits](playwright-community/eslint-plugin-playwright@v1.5.4...v1.6.0)

Updates `eslint-plugin-unicorn` from 51.0.1 to 52.0.0
- [Release notes](https://github.com/sindresorhus/eslint-plugin-unicorn/releases)
- [Commits](sindresorhus/eslint-plugin-unicorn@v51.0.1...v52.0.0)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: lint
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: lint
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: lint
- dependency-name: eslint-plugin-jest
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: lint
- dependency-name: eslint-plugin-playwright
  dependency-type: direct:development
  update-type: version-update:semver-minor
  dependency-group: lint
- dependency-name: eslint-plugin-unicorn
  dependency-type: direct:development
  update-type: version-update:semver-major
  dependency-group: lint
...

Signed-off-by: dependabot[bot] <[email protected]>
@csouchet csouchet removed the skip CI e2e tests GitHub Actions do not run e2e tests (for Pull Requests) label Jan 7, 2025
@csouchet csouchet marked this pull request as ready for review January 7, 2025 16:31
@csouchet csouchet requested a review from tbouffard January 7, 2025 16:31
Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, this has been a long running task 🎉

It is going to take me some time to review the migration to the flat config and test that the configuration is working as before.
I will try to review the PR in the next days/weeks

test/bundles/jest.config.cjs Outdated Show resolved Hide resolved
@@ -110,7 +110,7 @@ const computeConfigurationForDevelopmentServerUsage = defaultBrowsers => {
const computeConfiguration = options => {
let configuration;
configuration =
(options.startWebServer ?? true) ? computeConfigurationForDevelopmentServerUsage(options.defaultBrowsers) : computeConfigurationForStaticUsage(options.defaultBrowsers);
options.startWebServer || true ? computeConfigurationForDevelopmentServerUsage(options.defaultBrowsers) : computeConfigurationForStaticUsage(options.defaultBrowsers);
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I don't know if this change is due to a new eslint rule but IMHO, this introduce a behavior change.

Previously, the value was true only if the startWebServer option was nullish (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing).
The logic has been changed here to use a logical OR operator (falsy check instead of nullish check). Now, if the startWebServer option is false, the computed value is true as well. So the check is completely useless as the condition is always evaluated to true.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, is due to the rule. Maybe, we need to disable this rule on .cjs file.

Copy link
Member Author

Choose a reason for hiding this comment

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

After checking, it's not a rule who fails, but the ECMA version used in ESLint. 
If we want to keep ??, we need to upgrade the used ECMA version. 
Maybe, it works with ESLint v8, because of a bug (or, there is a bug in ESLint v9).

Copy link
Member

@tbouffard tbouffard Jan 17, 2025

Choose a reason for hiding this comment

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

Yes this is weird, as you mentioned, we should have been forced to set ecmaVersion: 2020 (instead of 2018, the operator was introduced in 2020) in the former configuration.
Can we set it now to 2020 to be able to restore the usage for Nullish coalescing operator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the Nullish coalescing operator works fine with ECMA 2020.

@@ -14,7 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// eslint-disable-next-line import/no-unresolved -- The bpmn-visualization package may not have been built prior running eslint (it happens when running GitHub Actions)
Copy link
Member

Choose a reason for hiding this comment

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

thought: for me only, I will check the commit history to understand why we introduce it and why this seems no longer necessary.

@tbouffard tbouffard added the skip CI e2e tests GitHub Actions do not run e2e tests (for Pull Requests) label Jan 13, 2025
@csouchet csouchet marked this pull request as draft January 16, 2025 16:50
@csouchet csouchet marked this pull request as ready for review January 17, 2025 10:24
@csouchet csouchet requested a review from tbouffard January 17, 2025 15:08
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 (dev or runtime) skip CI e2e tests GitHub Actions do not run e2e tests (for Pull Requests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants