chore(deps): FE-454 Update to ESLint 9 #298
Conversation
|
|
||
| ## Usage | ||
|
|
||
| Add `@bigcommerce/eslint-config` to your project's ESLint configuration file. i.e.: |
There was a problem hiding this comment.
I imagine we actually want to keep this line.
| '@typescript-eslint/return-await': 'error', | ||
| '@typescript-eslint/unbound-method': 'off', | ||
| '@typescript-eslint/unified-signatures': 'error', | ||
| // The following rules are off mostly due to incompatibilities with their |
There was a problem hiding this comment.
Is this still accurate?
There was a problem hiding this comment.
Pull Request Overview
This PR updates the repository to support ESLint 9 with its new flat config format, replacing the legacy ESLint 8 configuration system. The migration includes converting all modules to ES modules ("type": "module"), replacing CommonJS with ES module syntax, and restructuring configuration files to use the flat config format.
Key Changes:
- Updated ESLint peer dependency from ^8.0.0 to ^9.0.0 across all packages
- Converted all configuration files from CommonJS to ES modules
- Restructured ESLint configs to use flat config format instead of the legacy .eslintrc format
Reviewed Changes
Copilot reviewed 33 out of 41 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated ESLint to 9.x, added module type, modified test script for ES modules |
| packages/eslint-plugin/package.json | Updated ESLint peer dependency to 9.x and added module type |
| packages/eslint-config/package.json | Updated ESLint peer dependency to 9.x, added module type, replaced patch dependency with @eslint/js and globals |
| packages/eslint-plugin/index.js | Converted to ES modules with flat config format for plugin configuration |
| packages/eslint-plugin/rules/jsx-short-circuit-conditionals.js | Converted to ES modules and removed deprecated config properties |
| packages/eslint-plugin/tests/jsx-short-circuit-conditionals.spec.js | Converted to ES modules with compatibility handling for default exports |
| packages/eslint-config/index.js | Converted to async ES module returning flat config array |
| packages/eslint-config/configs/*.js | Converted all config files to ES modules with flat config format |
| packages/eslint-config/utils/index.js | Converted to async ES module using dynamic imports |
| packages/eslint-config/test/*.spec.js | Updated test files to ES modules with proper async handling |
| packages/eslint-config/prettier.config.js | Converted to ES module export |
| packages/eslint-config/jest.config.js | Added transform config for ES modules |
| eslint.config.js | New flat config file replacing .eslintrc.js |
| README.md | Updated documentation for ESLint 9 migration |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| recommended: true, | ||
| schema: [], // No options for this rule | ||
| docs: { | ||
| description: 'Disallows usage of string / number while short-circuiting jsx', |
There was a problem hiding this comment.
Corrected spelling of 'Disallows' to 'Disallow' to match standard rule description convention.
| description: 'Disallows usage of string / number while short-circuiting jsx', | |
| description: 'Disallow usage of string / number while short-circuiting jsx', |
| description: 'Disallows usage of string / number while short-circuiting jsx', | ||
| suggestion: true, | ||
| recommended: true, | ||
| schema: [], // No options for this rule |
There was a problem hiding this comment.
[nitpick] The comment 'No options for this rule' is redundant since an empty schema array already indicates this. Consider removing the comment to reduce noise.
| schema: [], // No options for this rule | |
| schema: [], |
| // In ES modules, we try to resolve the package from the current working directory | ||
| // Note: import.meta.resolve is experimental, so we'll use dynamic import instead | ||
| await import(pkgName); |
There was a problem hiding this comment.
The dynamic import will throw if the package is not in node_modules, but won't respect the current working directory context. This differs from the original require.resolve behavior with paths: [process.cwd()]. Consider using import.meta.resolve() with a try-catch if available, or document this behavioral change.
| ], | ||
| "@typescript-eslint/no-duplicate-enum-values": [ | ||
| "error", |
There was a problem hiding this comment.
this change in the way severity is reported is a bit unfortunate, any chance we can either prep the snapshots on a previous commit by converting "error", "warning" and "off", to 2, 1, and 0, or make the serializer bring back "error", "warning" and "off"? at least for this PR, we can remove it and update the snapshots on a follow up.
There was a problem hiding this comment.
I am happy to go do this, can you give me some instructions on how to do so? Do I just need to regenerate the snapshots on master or provide some actual instruction to use the numeric format?
There was a problem hiding this comment.
was just thinking on a quick and dirty search and replace in the original snapshot + commit before the new one, so looking at the second commit made spotting the differences easier
There was a problem hiding this comment.
Ok, so I ran find/replace in the snapshot files to swap error with 2, warn with 1 and off with 0 as a separate commit, and then the rest of the updates. It shrunk the diff slightly but there's still a lot.
PascalZajac
force-pushed
the
FE-454
branch
2 times, most recently
from
fd87d34 to
71d0856
Compare
|
Any estimate on when this upgrade will be available? I'm currently converting our Catalyst project to Nextjs 16 and ESLint 9 but getting a little stuck on the BigCommerce configs. |
|
@oliverdowie thanks for the question! It actually helps to know there are other people out there interested / waiting on this. We'll aim to get it finalised next week. |
What & Why?
Update this repository to be compatible with (and indeed require) ESLint 9, so we can keep moving into the glorious future of JS.
Examples
The tests themselves verify this change. Hopefully.