vuejs / eslint-config-typescript

ESLint configuration for Vue 3 + TypeScript projects
MIT License
129 stars 30 forks source link

fix: add overrides from eslint-recommended also for .vue files #40

Closed DrJume closed 2 years ago

haoqunjiang commented 2 years ago

🤔 This breaks the allow-js test case: https://github.com/vuejs/eslint-config-typescript/blob/8c3628cc43ce93c197d1f6b749712d2c0b2ebe4e/test/fixtures/allow-js/App.vue#L2-L3 (should fail on no-const-assign, but was overridden)

I've thought about adding a dedicated allow-js ruleset, but after reading the actual rules, I think it would be easier to revert this PR.

DrJume commented 2 years ago

Yes, I can see your point. The reason for this PR was, because I didn't like the double errors. I also think to remember that some of the ESLint warnings did not behave well with TS, creating false positives, but I am not sure I remember this correctly.

It would be awesome if we could match the .vue file but with a lang: [ts, tsx] parameter. Would this be possible with the vue-eslint-parser?

haoqunjiang commented 2 years ago

As far as I know, it's not possible due to limitations of eslint-plugin-vue (and maybe ESLint too) https://github.com/vuejs/eslint-plugin-vue/issues/1523#issuecomment-868368884

haoqunjiang commented 2 years ago

I also think to remember that some of the ESLint warnings did not behave well with TS, creating false positives, but I am not sure I remember this correctly.

Yeah, there are, but not the ones in the eslint-recommended ruleset.

It's in recommended, and when such a rule gets disabled, a corresponding typescript-eslint rule will be enabled: https://github.com/typescript-eslint/typescript-eslint/blob/dc1f9309cf04aa7314e758980ac687558482f47f/packages/eslint-plugin/src/configs/recommended.ts#L11-L12

DrJume commented 2 years ago

I think that if you use @vue/eslint-config-typescript every <script> should have lang="ts". Because mixing js and ts is not best practise. I would try to make a opt-out config, only if you mix js and ts.

haoqunjiang commented 2 years ago

I thought about offering an opt-out config too. But now I think it would require a more convincing motivation than the double-error issue.

There will always be projects with allowJs turned on (most likely are projects in the transition from JS to TS). It'll be too troublesome to offer them yet another choice with only subtle differences, yet we have to document these, which, many might not even read.

So, I don't want to bring in more maintenance burden and make newcomers harder to get into the ecosystem, unless there's absolute necessity.