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

ESLint v9 and flat config #774

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

ESLint v9 and flat config #774

wants to merge 3 commits into from

Conversation

spence-s
Copy link
Contributor

@spence-s spence-s commented Jan 14, 2025

My version of xo with flat config support.

It's a complete re-write obviously here in TS and this is the initial PR WIP, let me know what you think.

It seems to work pretty darn well for me so far as a drop replacement in the repos I have tested it on and it works out of the box with the vscode extension.

It also exports a createEslintConfig function, for users who'd rather use eslint directly but get (almost) all of the goodies from using xo cli directly.

The docs probably need work still and test coverage could definitely be better, I think some features are probably gone as well, I know I didn't add webpack import resolver yet and probably a few other things, so let me know if its missing something you feel is important.

You can test out a beta version https://www.npmjs.com/package/@spence-s/flat-xo which I will deprecate if this PR is accepted.

easy to replace in package.json "xo": "npm:@spence-s/flat-xo@latest"

@spence-s
Copy link
Contributor Author

spence-s commented Jan 29, 2025

I have experimented with adding a react flag to the beta version, Not included yet in this PR, which will enable react linting extremely easy. I really like it.

If we feel this is something we are ok with, I will add it to this PR as well.


We can also discuss #777 here I am open to dropping formatting support prettier while supporting prettier compatibility as an option.


@sindresorhus I know this is a big PR and difficult to figure out the right approach or if we want to support the new implementation or design decisions that I made. If you feel you'd like a different approach to getting this merged in or accomplished, I am open to it.

perhaps adding the flat-config behind a flag in the current CLI would work and make it a safer change. The reason I didn't do that as step 1 is because it will be a bit odd with the JS/TS boundary. It's also tough as we'd need to support multiple versions of the same dependencies in 1 repo. which may go poorly.

If you'd like, I will try to work on getting this PR to a more "integrated" and safe state for merging, but if you have suggestions for this let me know.

@fregante
Copy link
Member

If we feel this is something we are ok with, I will add it to this PR as well.

Given the size of the PR, I'd keep that for a follow-up instead.

@sindresorhus
Copy link
Member

sindresorhus commented Feb 6, 2025

Thanks for working on this and sorry about the slow response.

I think this is the right approach. Better to rip off the bandaid than complicate things with flags.


Is it not possible to support config in package.json anymore?

Pretty much all I used it for is to disable some rules, so maybe we could support a limited set? Like only disabling rules?


Before we eventually merge this, it would be great if you could run a script to convert indentation to tab-indentation. I could do it too, but then I would create noise for all the changes you did.

@sindresorhus
Copy link
Member

I have experimented with adding a react flag to the beta version, Not included yet in this PR, which will enable react linting extremely easy. I really like it.

I'm open to it, but should definitely be in a follow-up PR.

@spence-s
Copy link
Contributor Author

spence-s commented Feb 6, 2025

Is it not possible to support config in package.json anymore?

Pretty much all I used it for is to disable some rules, so maybe we could support a limited set? Like only disabling rules?

Sure! I had been debating this anyway I think its a good value add for the xo cli/editor extension to be able to use package.json.

We can support any parts of the config that are serializable, so basically everything except plugins I think and some language settings maybe.

So supported config types will be:
xo.config.js
xo.config.cjs
xo.config.mjs
xo.config.ts
xo.config.cts
xo.config.mts

package.json

optionally it would be trivial to add xo.config.json as well if we wanted.

I think its nice to give users a better reason to stick with xo eco system entirely since this PR gives them an easier way to integrate xo with eslint directly.

@spence-s
Copy link
Contributor Author

spence-s commented Feb 6, 2025

Before we eventually merge this, it would be great if you could run a script to convert indentation to tab-indentation. I could do it too, but then I would create noise for all the changes you did.

Yes, I'll take care of this as well.

@sindresorhus
Copy link
Member

We can support any parts of the config that are serializable, so basically everything except plugins I think and some language settings maybe.

But should we? I'm mainly interested in it for quick workaround like disabling/overriding some rules. If users needs to change a lot, it's probably better to create a config file. And also, by supporting many but not all things in package.json, it can create some confusion.

@fregante
Copy link
Member

As for the config, my advice is to ship a baseline XO that doesn't support much config at all, and then iterate on that.

As part of this PR, my advice would be to support the xo field in package.json, but just for rules, overrides, and XO flags, but not extends, plugins, and all that. You do not want to recreate eslintrc.

An xo.config.js file, if necessary, can come later.

@spence-s spence-s force-pushed the flat-config branch 2 times, most recently from 19ec066 to dda9495 Compare February 27, 2025 23:33
@spence-s spence-s force-pushed the flat-config branch 2 times, most recently from 7eba9b7 to a5c5e92 Compare February 28, 2025 20:58
@spence-s
Copy link
Contributor Author

spence-s commented Mar 1, 2025

Ok I think this is truly ready for review now and I have added back the stdin features, more tests, and simplified the dev configuration files better.

And also, it was too difficult to NOT include the new options I have added to this PR and maintain the fork where I do a lot of manual beta testing. So apologies for the extra overhead here.

Here is a running change log of things and important info to keep in mind for reviewing:

Changelog

New Features

  • Adds a static xoToEslintConfig method which can be used in an eslint.config.js file
    • This function will not do any lookup for the user, so it will be harder to use some of the features of xo, and it will not handle auto ts lookup nor prettier lookup.
  • exports the base xo config from xo/config ie… import {config} from 'xo/config' in an eslint.config.js file
  • new CLI and config options
    • react - Adds a base react option which adds eslint-config-xo-react to the selected files
    • prettier 'compat' Adds a new option for prettier, where if you set the option to the string "compat" it will simply turn off all the prettier affected @Stylistic rules using eslint-config-prettier otherwise it works the same as it does currently.
    • Allows turning off the automatic ts handling as an optimization (it is on by default) with a new config options ts
  • Allows ts files as stdin echo "const x: boolean = true" | xo --stdin --stdin-filename=stdin.ts
  • stdin option now defaults file names to stdin.js and no longer requires the --stdin-filename option
  • Only accepts configuration through an xo.config.{js,cjs,mjs,ts,cts,mts} file
    • open to supporting other methods, but I personally only use these configs. package.json could be handled in a number of different ways if we wanted to say only support the cli arguments + rules object - or make it a flat config array that only supports serializable features of the config array. Happy to implement if desired.

Removal of Features

  • Drops the node engines check from xo as they were only active in engines that are unsupported.
  • Drops auto webpack lookup and webpack import resolution (feels a bit niche, but would be willing to support still)
  • support for custom cli reporter removed (easy to add back if we wanted, didn't feel important) Now implemented
  • Removed CLI options
    • plugin no longer makes sense, still configurable with config
    • extend no longer makes sense, still configurable with config
    • env this no longer makes sense, still configurable with config
    • global this could potentially be added back but I have never used it from cli, still configurable with config
    • node-version removed because all of the node stuff was for unsupported node versions
    • extension could potentially be added back, has limited value from cli, can still be configured with config
    • reporter this could be added back but feel like its relatively unused and requires a potentially flaky lookup open to adding this back now and will likely in the future regardless

CLI and Change Summary

In general, this version of xo is much "thinner" in that we really just barely abstract over ESLint. In doing so, we make the config much more flexible for users, and really there is no feature loss when moving from eslint to xo. The tradeoff is that the xo cli itself does not add as much capabilities but it currently does have a few advantages that it.

  • easily accepts ts configuration (xo.config.ts) with no extra steps
  • will handle prettier lookups if you use that option and integration with prettier will be much smoother
  • handles stdin and stdin ts files
  • automatic tsconfig resolution and typeaware linting automatically

If I think of more changes I will continue to add here, but this covers the major bases I feel.

Also note, I have been using this for the past few months and its been working great, I think it is ready for a release, but happy to make adjustments if you see issues.

Beta test this PR right now in your own repos

If you'd like to test right now, my fork is published here and I'm keeping this PR and the fork synced: https://www.npmjs.com/package/@spence-s/flat-xo

To test with the fork npm install xo@npm:@spence-s/flat-xo and let me know if you experience issues, we already have a few beta users that have been using it and the xo vscode extension is using it for linting as well as all the repos I have been working in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants