-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
feat(create-vite): add eslint.config.js to React templates #12860
Conversation
Run & review this pull request in StackBlitz Codeflow. |
Thanks for this. I tested it and work as expected. I'm really surprised that we require to install to more packages for this, which is not mentioned in the doc that prefer to explain me what is object shorthand 🤦♂️ The documentation on this is so poor that I'm not sure we should push new Vite users that could be people still learning web basics into this. I will see what the react of team think of that. Thanks for the work, in any case this will be useful and merge when this become the official config format |
Hi! We decided to wait for eslint to switch the default config format to the new one before adding it to the template. |
The default switched to flat config with today's |
We still need to wait jsx-eslint/eslint-plugin-react#3699 and facebook/react#28313 to be resolved |
Hi @ArnaudBarre Shall we consider using the |
Yeah I think this is better for new users to get started with the new config at the cost of one more compat package. Each usage of the compat should prefix with a comment But first we should wait for the VS Code extension to support v9 without pre-release: https://github.com/Microsoft/vscode-eslint/blob/HEAD/CHANGELOG.md |
Thanks for the update, the only downside is that we can peer deps warning Edit: it seems they aye still conflicts, I test locally once resolved |
@ArnaudBarre All issues should be fixed now |
When I run bjorn@Bjorns-MacBook-Pro vite-project % npm i
npm error code ERESOLVE
npm error ERESOLVE unable to resolve dependency tree
npm error
npm error While resolving: vite-project@0.0.0
npm error Found: eslint@9.5.0
npm error node_modules/eslint
npm error dev eslint@"^9.5.0" from the root project
npm error
npm error Could not resolve dependency:
npm error peer eslint@"^8.56.0" from @typescript-eslint/parser@7.13.1
npm error node_modules/@typescript-eslint/parser
npm error dev @typescript-eslint/parser@"^7.13.1" from the root project
npm error peer @typescript-eslint/parser@"^7.0.0" from @typescript-eslint/eslint-plugin@7.13.1
npm error node_modules/@typescript-eslint/eslint-plugin
npm error dev @typescript-eslint/eslint-plugin@"^7.13.1" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution. Do we want to use the alpha versions of |
I didn't know npm was that strict. I only got warning with bun/yarn. We will get peer deps issues for react plugins too, so I don't know what we can do |
Ah yeah for the JS template, I get: npm error Could not resolve dependency:
npm error peer eslint@"^3 || ^4 || ^5 || ^6 || ^7 || ^8" from eslint-plugin-react@7.34.3
npm error node_modules/eslint-plugin-react
npm error dev eslint-plugin-react@"^7.34.2" from the root project Since the TS template, doesn't have |
I only tested using yarn. Shall we do
in package.json ? |
After some research we can probably add the following field to package.json
|
Hi @ArnaudBarre is there anything else we should do in this merge request to move the ecosystem forward? |
Just some helpful info:
|
Here is a working config. The caveat is that import js from '@eslint/js'
import reactHooks from 'eslint-plugin-react-hooks'
import reactRefresh from 'eslint-plugin-react-refresh'
import reactJsxRuntime from 'eslint-plugin-react/configs/jsx-runtime.js'
import reactRecommended from 'eslint-plugin-react/configs/recommended.js'
import globals from 'globals'
import ts from 'typescript-eslint'
export default ts.config(
// ignore patterns (.eslintignore)
{ ignores: ['dist', 'eslint.config.js'] },
// misc
{ languageOptions: { globals: { ...globals.browser, ...globals.es2020 } } },
// typescript eslint
js.configs.recommended,
...ts.configs.recommended,
{
languageOptions: {
parserOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
project: ['./tsconfig.json', 'tsconfig.app.json', './tsconfig.node.json'],
tsconfigRootDir: import.meta.dirname
}
}
},
// react
reactRecommended,
reactJsxRuntime,
{ settings: { react: { version: 'detect' } } },
// react hooks
{
plugins: { 'react-hooks': reactHooks },
rules: { ...reactHooks.configs.recommended.rules }
},
// react refresh
{
plugins: { 'react-refresh': reactRefresh },
rules: {
'react-refresh/only-export-components': ['warn', { allowConstantExport: true }]
}
},
// linter
{ linterOptions: { reportUnusedDisableDirectives: 'error' } }
) {
"@eslint/js": "9.7.0",
"@types/eslint__js": "8.42.3",
"@types/node": "20.14.11",
"@types/react": "18.3.3",
"@types/react-dom": "18.3.0",
"@vitejs/plugin-react-swc": "3.7.0",
"eslint": "9.7.0",
"eslint-plugin-react": "7.34.4",
"eslint-plugin-react-hooks": "4.6.2", // not using canary
"eslint-plugin-react-refresh": "0.4.8",
"globals": "15.8.0",
"typescript": "5.5.3",
"typescript-eslint": "7.16.1",
"vite": "5.3.4"
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After discussion with @bluwy we think it's better to switch to full v9 compatibility, remove the compat layer and use the canary version of react hooks plugin for now.
Also using the new typescript-eslint
with the typed config would be good for the TS package: https://typescript-eslint.io/packages/typescript-eslint/#advanced-usage
Edit: Sorry for the long answer, we can take over the PR if don't have time to update it in the next days, no problem for us, thanks a lot for the initial work and exploring the compat option!
@ArnaudBarre Sounds good to take over this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry totally forgot about it!
Description
Hi @ArnaudBarre As a follow up on #12801 I have helped to create an example using the latest eslint.config.js format of config
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).