eslint / eslint

Find and fix problems in your JavaScript code.
https://eslint.org
MIT License
24.66k stars 4.44k forks source link

Implement Flat Config #13481

Open nzakas opened 4 years ago

nzakas commented 4 years ago

This issue describes the implementation plan for eslint/rfcs#9, which will take place in several phases:

Phase 1: Extract current config system

Phase 2: Implement flat config with eslintrc compatibility

Phase 3: Compatibility testing

Phase 4: General availability (v9.0.0)

Phase 5: Remove eslintrc (v10.0.0)

nzakas commented 3 years ago

Update: Most of the CLIEngine tests are so tightly coupled to the existing file structure (overriding and stubbing require imports and then trying to use an in-memory file system) that there's just no way I can get all of the tests to pass. I suspect that the tests aren't actually testing what we think they are due to this complexity. It seems my only path forward is to leave the eslintrc functionality in place in the eslint repo and then make a copy in the eslintrc repo. We will just have to focus on removing the eslintrc functionality when we remove CLIEngine.

nzakas commented 3 years ago

I'm going to make one more pass but using a physical file system instead of an in-memory filesystem to see if I can disentangle the tests. If that doesn't work, I'll do a more minimal extraction and move on.

nzakas commented 3 years ago

Phase 1 is now compete. The old config system has been separated out into a separate package. That means I can now move on to phase 2. 🎉

SrBrahma commented 3 years ago

Sorry for making this question for such a long process, but, do you have an idea of when this will be done? I am really interested about the https://github.com/eslint/eslint/issues/3458 implementation, so I can have my simple all-in-one-ESLint config.

Some hours ago and while writing this, I have been developing this idea (which I don't think would worth creating a new Issue), which could be done via the eslint-config package.json scripts but really could be a ESLint default functionality:

npx eslint install @srbrahma -e ts react

ESLint is hugely awesome, but is also considerably time consuming to set up the environment and to update all the plugins for each project. There could be this single magic cli that would do everything, to quickly maintain and create new projects. It really doesn't seem to be too complicated to set this up, being the #3458 implemented. I think it would really take ESLint to a new level :)

nzakas commented 3 years ago

@SrBrahma It will be at least a year before we’ve transitioned completely to the new config system and probably around three months before you can use the new config system optionally. There are plenty of workarounds mentioned in #3458 in the meantime.

valtlai commented 3 years ago

Could eslint.config.cjs be supported too? This way "type": "module" can be used in package.json even though ESLint doesn’t support loading an ECMAScript module as a config file.

nzakas commented 3 years ago

@valtlai please open a separate issue for that request. This issue is just for tracking implementation of the RFC. Thanks.

valtlai commented 3 years ago

@valtlai please open a separate issue for that request. This issue is just for tracking implementation of the RFC. Thanks.

Done, that’s issue #14170.

nzakas commented 2 years ago

An update on the approach: It turned out to be too difficult to modify the ESLint class to toggle between the new and old config systems, so instead I'm creating a new FlatESLint class that will be the basis of the new config system. cli.js will search for the new config file and use FlatESLint instead of ESLint if found. As a final step, we will remove the original ESLint class and rename FlatESLint to ESLint.

nzakas commented 2 years ago

It looks like I'm going to have to create a FlatRuleTester that mirrors RuleTester in the short term. There are just too many differences between flat config and eslintrc to allow for an easy switch inside of the current RuleTester. The plan is to implement FlatRuleTester as a temporary utility to allow people to test their rules with flat config. Eventually, we will delete the current RuleTester and rename FlatRuleTester to RuleTester.

nzakas commented 2 years ago

An update on FlatESLint: I actually have lintFiles() and lintText() working. The complicated part is dealing with ignored files. We are currently using FileEnumerator as a catch-all that does globbing, config calculation, and ignore handling, and that doesn't really work with flat config. So, I'm in the process of pulling that apart and trying to get the same behavior with FlatESLint. Unfortunately, it looks like we won't be able to get the exact same behavior because FileEnumerator deals with files in sequence while globby deals with them in parallel, so I'm doing my best to get as close as I can.

bradzacher commented 1 year ago

I think it would be a good idea to include @typescript-eslint in the Phase 3 list.

IMO onboarding the project as early as possible is a hard requirement for the workstream. Without ts-eslint's onboarding the majority of ESLint users won't able to transition to flat configs without needing to use the compatibility tooling.

For reference - by weekly download volume @typescript-eslint/eslint-plugin users make up over 65% of ESLint users and @typescript-eslint/parser users make up over 70% of ESLint's users. (plugin = 19.2m / parser = 21m / eslint = 28.8m)

It would be a real shame if most users' first experience with new config system involves reaching for the compatibility APIs. Making sure that doesn't happen should be a top priority!

Feel free to chat with us (cc @JoshuaKGoldberg and @JamesHenry)!


Separately but very related to the above - I'd also suggest including eslint-plugin-import for the same reasons given that it is similar in size (17.1m - ~60%).

kirill-konshin commented 1 year ago

Could not agree more @bradzacher. The success of tools like Next.js, Jest, Prettier and even Create React App proves that many users want simple tools, they need fast onboarding and prefer opinionated presets.

It's great to have a configurable tool, but for majority of users thousands of options are overwhelming. People simply do not have time to dig into nuances, they need a solution.

In my opinion, ESLint should provide some dead easy way to kick off a fresh project with all necessary dependencies. Something like npm install eslint && eslint init recommended. Look at the guide https://eslint.org/docs/latest/user-guide/getting-started and https://eslint.org/docs/latest/user-guide/configuring/configuration-files-new — and imagine what most users think while reading this...

nzakas commented 1 year ago

@bradzacher will do. The intent is to use phase 3 to test the ecosystem in general.

@kirill-konshin this is off topic for this issue, but you can use npm init @eslint/config. It’s listed right on eslint.org. 😄

Shinigami92 commented 1 year ago

Thanks a lot to @sxzz since eslint-define-config v1.8.0 Flat ESLint Config is officially supported by my plugin :rocket:

mahnunchik commented 1 year ago

Any news?

nzakas commented 1 year ago

@mahnunchik you can look at the checklist in the original comment to see the current progress.

karlhorky commented 1 year ago
  • Work with eslint-plugin-react to switch configs to new format and ensure it works correctly

@nzakas now that the new config format has been added in https://github.com/jsx-eslint/eslint-plugin-react/pull/3429, should this checkbox in Phase 3 be checked?

ljharb commented 1 year ago

eslint-plugin-react v7.32.0 has been released, with flat config support - but only less than 12 hours ago. It's probably wise to wait a bit longer to make sure it actually does work correctly :-)

CallMeLaNN commented 1 year ago

Guys, I hope I ask in the right place and time for the phase 3. What will be the best convention to export shared config for plugins maintainer to follow?

I hope it can be consistent so plugin user can set recommended shared config in a same way across plugins.

I afraid, the old plugin configs is not compatible be used directly because it may have extends, overrides and plugins type array of string instead of object in the shared config. If we upgrade the configs, I don't think we can make it "hybrid" to support both old and new config at the same time. It must be exposed trough a different way to make it backward compatible.


Examples

The eslint-plugin-react approach above looks promising. The new shared config exposed trough a deep import path:

const reactRecommended = require('eslint-plugin-react/configs/recommended');
// The old configs is still in here: reactRecommended.configs.recommended for backward compatibility.

module.exports = [
  …
  reactRecommended,
  …
];

However the Configuration Files New doc recommends like below. Similar like in the RFC Plugins Specifying Their Own Namespaces which I believe it will be breaking for some plugins 🤔

// doc
import jsdoc from "eslint-plugin-jsdoc";
export default [
    // configuration included in plugin
    jsdoc.configs.recommended,
    // other configuration objects...
];

// RFC
module.exports = [
    require("eslint-plugin-example").configs.recommended,
    // ...
];
nzakas commented 1 year ago

This is a tracking issue and not the best place to ask for help. Please open a discussion instead.

CallMeLaNN commented 1 year ago

This is a tracking issue and not the best place to ask for help. Please open a discussion instead.

My intention was to get a chance to help or send PR for some plugins since I already study the code of the flat configs on both Eslint and some plugins. It could be the way to start contribute to the community on the phase 3.

Anyway, in that case, you may update somewhere about the concern above whenever Eslint team is ready.

nzakas commented 1 year ago

@CallMeLaNN that's great, we'd appreciate the help...but this is still not the right place for that type of question. Happy to continue in a separate discussion.

homersimpsons commented 1 year ago

This looks really promising and a huge step forward.

I cannot seem to find an answer to my issue: How to use .gitignore as a default ignore file ?

Previously there were the --ignore-path command line option, I would expect this command line to be migrated to the flat config but cannot seem t find anything about this (https://eslint.org/blog/2022/08/new-config-system-part-2/, https://eslint.org/docs/latest/use/configure/configuration-files-new).

Furthermore it looks like the .eslintignore file does not work anymore.

What is the recommended way ? I think a documentation change / migration guide may be required to clarify this.

Solution 1: copy-paste and maintain

  1. Re-introduce .eslintignore
  2. cp .gitignore .eslintignore
  3. maintain both files

Solution 2: symbolic link

  1. Re-introduce .eslintignore
  2. ln -s .gitignore .eslintignore

Solution 3: configuration option

Introduce an "ignorePath" option that would read a .gitignore like file and apply it

Solution 4: configuration utilities

Stick with the "ignores" configuration, but provide a utility function to easily convert .gitignore to "ignores"

...
    "ignores": ...fromGitignore('.gitignore'),
...

Solution 5: do not change anything

Let people re-define manually their .gitignore in "ignores" and keep them in sync

My point of view

I guess the most consistent solution is the 4th one (configuration utilities). The user may be responsible to read the file as a string, but I guess this use case is pretty common so doing it within the utility may reduce boilerplate.

The 3rd solution may be easier to discover, but may introduce some inconsistencies

Other solutions should be avoided to me.

mdjermanovic commented 1 year ago

@homersimpsons "ignores" can also contain custom functions that should return true if the given file/directory path should be ignored. For gitignore-like patterns, you can use the ignore package. Config file that loads a .gitignore file could look like this:

// eslint.config.js

import ignore from "ignore";
import { readFileSync } from 'node:fs';
import path from 'node:path';

const myIgnore = ignore().add(readFileSync(".gitignore", "utf-8"));

export default [
    {
        ignores: [
            absolutePath => { 
                const relativePath = path.relative(".", absolutePath);

                if (relativePath === "") {
                    return false;
                }

                return myIgnore.ignores(relativePath);
            }
        ]
    },
    {
        // other configurations
    }
];
nzakas commented 1 year ago

Folks - if you have questions about flat config, please open a discussion rather than commenting here. This issue is for tracking purposes only.

mdjermanovic commented 10 months ago
  • Remove context.parserPath, context.parserOptions, context.globals in Linter

Does context.globals exist?