-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Support ESLint 9 #705
Comments
Hi ! Eslint FlatConfig full support is the task that is giving us the biggest headache, and we can't go without it because we will have to do it when the deprecated What NX is doing right now is sharing a CLI command to convert
Might be a starting point / inspiration for your plugin ? Everything is based on the :
But there are some dirty workaround if in the future Example of an Ugly workaround we had do to, to prevent the fact that both antfu and our custom eslint lint were exporting (extending in fact) the react plugin:Error: key "plugins": Cannot redefine plugin "react". const { FlatCompat } = require('@eslint/eslintrc') const js = require('@eslint/js') const ts = require('@typescript-eslint/eslint-plugin') const hooks = require('eslint-plugin-react-hooks') const next = require('@next/eslint-plugin-next') const nx = require('@nx/react') const a11y = require('eslint-plugin-jsx-a11y') The fact that there are no clear information across the web about FlatConfig may become the highest wall atm 😞. In the NX context, the fact that eslint FlatConfig CLI command is in the "tips'n'tricks" section of their documentation is not really reassuring either... Also, some HUGE chunks of mandatory resources like globals are missing clear documentation and the information found online is not the freshest. On our side, the full FlatConfig support is coming form the fact that we want to use tools like https://github.com/antfu/eslint-config that are already fully supporting the eslint FlatConfig spec. Don't know if its really helpful but I hope you will find what you need inside the NX migration script or :
|
A few comments on this feature:
The internal base config would also need to be represented as a flat config. My suggestion would be something like tseslint.config which exposes flat configs that can be spread as well as a helper function to handle nested flat configs. We could even re-export it: https://typescript-eslint.io/packages/typescript-eslint#config |
@TzviPM As much as possible, oneRepo should avoid creating custom configurations for other tools. I don't see any reason to do anything special with regards to how the ESLint configurations are constructed. The current plugin does not do any referencing or checks on the configuration file. The only thing that may or may not be necessary is to parse the new eslint flat config in order to read the ignored files (done here, with test) in order to avoid sending already ignored files to the linter. That being said, I'd be fine with recommending using the .gitignore solution and encourage using It's been too long since I've looked at the problems that ESLint 9 gave me, but I think most of them revolved around third-party/custom rules and plugins, which made it a non-starter for even testing within the oneRepo git repository itself. |
@paularmstrong yes, sorry. To clarify, I was referring to the internal package that’s used by the onerepo eslint config itself. That package will need to export rules as an array and then our eslint.config.js file will need to spread it into its own config. It seems most of the 3rd party plugin issues are resolved. One of the big ones was the import plugin that was blocking many downstream plugins that relied on it |
If you’re open to a version that drops eslint 8 support, I can put up a PR. Otherwise, we need to figure out how we want to allow backwards compatibility. I think tools like nextjs are setting an environment variable ESLINT_USE_FLAT_CONFIG=false based on some heuristic. We could provide a configuration option like ‘useFlatConfig’ that could be set to false whereby we could set the corresponding environment variable. Thoughts? |
I'm a little hesitant to fully drop eslint 8 support, as I know of a lot of teams (myself included) that are having a hard time justifying putting in the work for the update. I prefer to avoid environment variables on configurations as much as possible, and your alternative suggestion for a config option on the plugin itself would make more sense anyway, given that you want to ensure it's set specifically for the repo itself. I can think of two alternatives, but still prefer your suggestion. Adding those here just in case:
Neither of these alternatives is as performant or as obviously strict as an explicit configuration option.
Ah yes! Thanks for mentioning… I think this was my biggest blocker as well. Good to know it's been updated |
The environment variable is part of eslint 9 itself. We can keep eslint as a peer dependency with version "^8 || ^9" and then keep our own understanding of which eslint version and which config file format. For the CLI commands, I believe the We can provide out own |
Checklist
Problem
@onerepo/plugin-eslint
does not work with ESLint ^9Suggested solution
May need special handling between the two versions or else a new major version that only supports ESLint 9
The text was updated successfully, but these errors were encountered: