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 configs #2407

Merged
merged 21 commits into from Mar 25, 2024
Merged

feat: support flat configs #2407

merged 21 commits into from Mar 25, 2024

Conversation

aladdin-add
Copy link
Contributor

@aladdin-add aladdin-add commented Feb 19, 2024

the main changes:

  • flat configs are exported as flat/*.
  • flat configs are exported as arrays.
  • configs naming:
    • flat/recommendedis for vue 3.
    • flat/vue2-recommended is for vue 2.

resubmit #2319.
fixes #1291

@ganondev
Copy link

ganondev commented Feb 19, 2024

Hi, just wanted to share some of the challenges I had getting this to work in the context of my vue3+typescript project.
The work I did was from the original @conradhale fork, but I checked out @aladdin-add as well and it seems to be missing one thing I had to add:

  • Setting plugins: { vue: module }, for every flat config option wasn't working out. I ended up changing the generator to set a recursive reference for each flat config at the bottom of index.js after declaring the export object. Here is an example:
module.exports.configs['flat/base'].plugins.vue = module.exports;

https://github.com/ganondev/eslint-plugin-vue/blob/master/tools/update-lib-index.js#L62

This is what the eslint.config.js for the project using this looks like with that update:

import eslint from '@eslint/js';
import tseslint from 'typescript-eslint';

import vue from 'eslint-plugin-vue';


export default [
  {
    ignores: [
      'api/',
      'node_modules/',
      'dist/',
      '.yarn/',
      '.pnp.cjs',
      '.pnp.loader.mjs'
    ],
  },
  {
    ...eslint.configs.recommended,
    languageOptions: {
      ...eslint.configs.recommended.languageOptions,
      globals: {
        require: 'readonly',
        console: 'readonly',
      }
    },
  },
  ...tseslint.configs.recommended,
  {
    files: ['**/*.vue'],
    ...vue.configs["flat/vue3-recommended"],
    languageOptions: {
      ...vue.configs["flat/vue3-recommended"].languageOptions,
      parserOptions: {
        ...vue.configs["flat/vue3-recommended"].languageOptions.parserOptions,
        parser: '@typescript-eslint/parser'
      }
    },
  },
  {
    rules: {
      indent: ['error', 2],
    }
  },

];

Please let me know if this is how this is intended to look. It would be nice if I could parameterize the typescript parser option in, instead of flatting out down to the shared parserOptions.
Also, it's worth noting that with the other shared configs, the vue one must be the last one, otherwise the typescript or eslint config will attempt to process .vue and throw a fit. I think this should be documented, not sure how much control the vue plugin would have to be able to soften that.

Unfortunately, I can't put export change up against the active fork here since it's a different upstream, but I wanted to make sure this at least gets shared.

One last thing, taking inspiration from how tseslint tackled this, they seem to have completely duplicated the configurations to keep the flat exports separate from the legacy configs, unlike what is being done here where the flat config is in the same export. I realize this is somewhat of an overhaul, but I think it might be the right answer.

@aladdin-add
Copy link
Contributor Author

@ganondev thanks for trying it - it seems a bug, I've pushed a new commit, should be fixed now.

@aladdin-add aladdin-add changed the title feat!: support flat configs feat: support flat configs Mar 4, 2024
@aladdin-add aladdin-add marked this pull request as ready for review March 4, 2024 07:45
@aladdin-add
Copy link
Contributor Author

friendly ping @ota-meshi

docs/rules/index.md Outdated Show resolved Hide resolved
lib/utils/config-helpers.js Outdated Show resolved Hide resolved
tools/update-docs.js Outdated Show resolved Hide resolved
tools/update-lib-index.js Outdated Show resolved Hide resolved
lib/index.js Outdated Show resolved Hide resolved
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests. I think this looks quite good already! I have just a few more minor nits, then this is ready to go from my side 🙂

If you still don't have enough, an improvement (for a follow-up PR) would be adding types for the configs, see eslint/eslint#18093 (comment). But I can totally understand if you don't want to put more effort into this, so everyone else reading this, feel free to go ahead! I think that adding non-typed flat config support is the step with the most impact though, so thanks a lot for putting this forward!

tools/update-lib-index.js Show resolved Hide resolved
tools/update-lib-index.js Outdated Show resolved Hide resolved
tests/lib/configs/flat.js Outdated Show resolved Hide resolved
aladdin-add and others added 2 commits March 20, 2024 10:21
Co-authored-by: Flo Edelmann <git@flo-edelmann.de>
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

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.

Support for "Flat Config"
4 participants