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

feat!: support flat config #81

Merged
merged 41 commits into from Apr 4, 2024
Merged

feat!: support flat config #81

merged 41 commits into from Apr 4, 2024

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented Jan 10, 2024

Implementing flat support is a good opportunity that allows us to rewrite and simplify its implementation. This PR implements an initial version for gathering feedback. There are some challenges and issues:

  1. Some plugins temporarily do not support flat config. Should we consider using @eslint/eslintrc for compatibility?
  2. env: node/browser is no longer supported in flat config. Should we introduce the globals package to support this feature?
  3. --config was previously used as a shortcut for using shareable configurations. There seems to be no reliable way to determine if it is in flat format. Can we assume that it is?

fixes #51

Remaining tasks:

  • vue.js support
  • ts support
  • --config options
  • docs updates

🧪for testing

$ npm init @weiran.zsd/eslint-config@latest

@aladdin-add aladdin-add changed the title feat: support flat config feat!: support flat config Jan 10, 2024
@nzakas
Copy link
Member

nzakas commented Jan 11, 2024

1. Some plugins temporarily do not support flat config. Should we consider using `@eslint/eslintrc` for compatibility?

If they work correctly with FlatCompat, then yes, I think this makes sense.

2. `env: node/browser` is no longer supported in flat config. Should we introduce the `globals` package to support this feature?

Yes.

3. `--config` was previously used as a shortcut for using shareable configurations. There seems to be no reliable way to determine if it is in flat format. Can we assume that it is?

If the CLI is running in flat config mode, then --config will assume the file is in flat config format.

@nzakas
Copy link
Member

nzakas commented Feb 6, 2024

@aladdin-add what's the status on this? I'd really like to roll out this change soon, so if you need help, please us know.

@aladdin-add
Copy link
Member Author

aladdin-add commented Feb 7, 2024

@nzakas I've added the remaining tasks in the PR desc.

@nzakas
Copy link
Member

nzakas commented Feb 7, 2024

Thanks, and do you need help finishing? As I mentioned, we'd like to get this out ASAP.

@aladdin-add
Copy link
Member Author

The main blocker is many community plugins/configs do not support it yet. (vue, airbnb, xo as of now).

eslint/eslint#18093

I'd like it to be officially supported before we merge this.

@nzakas
Copy link
Member

nzakas commented Feb 19, 2024

We can't really afford to wait for all plugins to be updated because v9.0.0 is already in beta and it doesn't make sense to have us generating eslintrc files anymore. Can't we just use FlatCompat for now?

aladdin-add and others added 3 commits March 25, 2024 00:04
Co-authored-by: Francesco Trotta <github@fasttime.org>
Co-authored-by: Francesco Trotta <github@fasttime.org>
fasttime
fasttime previously approved these changes Mar 25, 2024
Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Leaving open for @mdjermanovic to review, as requested.

@aladdin-add
Copy link
Member Author

friendly ping @mdjermanovic

@aladdin-add
Copy link
Member Author

eslint-plugin-vue has landed the official supports: https://github.com/vuejs/eslint-plugin-vue/releases/tag/v9.24.0

lib/config-generator.js Outdated Show resolved Hide resolved
fasttime
fasttime previously approved these changes Mar 30, 2024
@mdjermanovic
Copy link
Member

Is it intentional that we no longer show the list of dependencies that are going to be installed and ask the user for confirmation to install them?

@aladdin-add
Copy link
Member Author

yes, it's relying on installing to update the package.json, and it seems not especially useful. I can add it back if you like it tho. :)

@mdjermanovic
Copy link
Member

yes, it's relying on installing to update the package.json, and it seems not especially useful. I can add it back if you like it tho. :)

I think we should add it back to let the user know what packages are going to be installed and provide a way to generate the config file without installing them (e.g., in case the user is using some package manager that isn't listed).

@nzakas
Copy link
Member

nzakas commented Apr 2, 2024

I agree, I think showing the dependencies to install before installing them is useful and should be included.

@aladdin-add
Copy link
Member Author

aladdin-add commented Apr 3, 2024

@mdjermanovic I've pushed a commit: 0a4c547

nzakas
nzakas previously approved these changes Apr 3, 2024
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Leaving open for @mdjermanovic to verify.

aladdin-add and others added 2 commits April 4, 2024 16:10
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 54ac1f2 into main Apr 4, 2024
8 checks passed
@mdjermanovic mdjermanovic deleted the issue51 branch April 4, 2024 09:22
@github-actions github-actions bot mentioned this pull request Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

feat: support the new flat config
4 participants