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

fix: generated vue-ts config #64

Merged
merged 3 commits into from Jun 1, 2023
Merged

fix: generated vue-ts config #64

merged 3 commits into from Jun 1, 2023

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented May 30, 2023

fixes eslint/eslint#17221

it's a bug when choosing vue+ts. to repro:

➜  $ npm init @eslint/config                                           
Need to install the following packages:
  @eslint/create-config@0.4.3
Ok to proceed? (y) y
✔ How would you like to use ESLint? · problems
✔ What type of modules does your project use? · esm
✔ Which framework does your project use? · vue
✔ Does your project use TypeScript? · No / Yes
✔ Where does your code run? · browser
✔ What format do you want your config file to be in? · JavaScript
The config that you've selected requires the following dependencies:

eslint-plugin-vue@latest @typescript-eslint/eslint-plugin@latest @typescript-eslint/parser@latest
✔ Would you like to install them now? · No / Yes
✔ Which package manager do you want to use? · npm

the generated config (.eslintrc.cjs):

module.exports = {
    "env": {
        "browser": true,
        "es2021": true
    },
    "extends": [
        "eslint:recommended",
        "plugin:vue/vue3-essential",
        "plugin:@typescript-eslint/recommended"
    ],
    "overrides": [
    ],
    "parser": "@typescript-eslint/parser",
    "parserOptions": {
        "ecmaVersion": "latest",
        "sourceType": "module"
    },
    "plugins": [
        "vue",
        "@typescript-eslint"
    ],
    "rules": {
    }
}

the cause is .vue files should be parsed by vue-eslint-parser, not @typescript-eslint/parser. Its documentation says that custom parsers should use parserOption.parser to config.
https://eslint.vuejs.org/user-guide/#how-to-use-a-custom-parser

the new generated config:

module.exports = {
    "env": {
        "browser": true,
        "es2021": true
    },
    "extends": [
        "eslint:recommended",
        "plugin:@typescript-eslint/recommended",
        "plugin:vue/vue3-essential"
    ],
    "parserOptions": {
        "ecmaVersion": "latest",
        "parser": "@typescript-eslint/parser",
        "sourceType": "module"
    },
    "plugins": [
        "@typescript-eslint",
        "vue"
    ],
    "rules": {
    }
}

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label May 30, 2023
@aladdin-add aladdin-add marked this pull request as draft May 30, 2023 03:40
@aladdin-add aladdin-add changed the title fix: vue config should be placed at the end of extends fix: generated vue-ts config May 30, 2023
@aladdin-add aladdin-add marked this pull request as ready for review May 31, 2023 02:57
@aladdin-add aladdin-add requested review from nzakas, mdjermanovic and a team May 31, 2023 02:57
lib/init/config-initializer.js Outdated Show resolved Hide resolved
Comment on lines 228 to 231
// The configuration of the framework plugins should be placed at the end of extends.
if (answers.framework === "react") {
config.plugins.push("react");
config.extends.push("plugin:react/recommended");
Copy link
Member

Choose a reason for hiding this comment

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

After this change, plugin:react/recommended would come after airbnb, so some rules that airbnb explicitly turns off (for example, react/jsx-key because it has too many false positives) would be enabled. Perhaps there's actually no need for plugin:react/recommended when airbnb is selected?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, but IMHO, the recommended config should always be safe to use - if the rule has too many false positives, it should not be enabled in the recommended. :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it looks unusual that shareable config disables plugin's recommended rules, but I think that generally if the selected shareable config uses a plugin, then the shareable config should have precedence over plugin's configs, i.e. we should just not use plugin's configs.

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 0be55af into main Jun 1, 2023
14 checks passed
@mdjermanovic mdjermanovic deleted the fix/vue-config branch June 1, 2023 11:43
@github-actions github-actions bot mentioned this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants