qwik icon indicating copy to clipboard operation
qwik copied to clipboard

fix:consistent type imports warning

Open harshmangalam opened this issue 2 years ago • 1 comments

What is it?

  • [x] Feature / enhancement
  • [ ] Bug
  • [ ] Docs / tests

Description

fix consistent type import warning

harshmangalam avatar Jan 15 '23 12:01 harshmangalam

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Thanks @harshmangalam for taking the time to create this PR 🙏 May i ask what the motivation for the made changes is? As far as i know, we shouldn't use type imports when ever possible which covers with the statement of andrewbranch (member of the typescript team) as he stated here https://github.com/microsoft/TypeScript/issues/39861#issuecomment-668131921. Many thanks for a quick reply and for your help pushing qwik forward 🙏

zanettin avatar Feb 16 '23 20:02 zanettin

Vs code warn when we import type without type keyword.

harshmangalam avatar Feb 16 '23 20:02 harshmangalam

Wow thanks a lot for the quick reply 🔥 🙏

I just checked it on latest main branch on my side vscode doesn't complain about it on the imports 🙈 Bildschirmfoto 2023-02-16 um 21 10 51

tsc: 4.9.5

zanettin avatar Feb 16 '23 20:02 zanettin

Your eslint is disabled hence you are not getting any warning.

harshmangalam avatar Feb 17 '23 16:02 harshmangalam

entry ssr tsx - Untitled (Workspace) - Visual Studio Code 17_02_2023 21_59_13

Typescript v4.9.5

harshmangalam avatar Feb 17 '23 16:02 harshmangalam

Thanks again for your feedback 🙏

My eslint is working and reports all issues as it should except the one you refer here 🙈 The mentioned rule @typescript-eslint/consistent-type-imports is not part of the recommended config of @[email protected] which is used on the default generated .eslintrc.cjs nor in any other referenced configs/inline config sets.

these presets are defined and as far as i can see non of them have the rule included 🙈 (pls correct me if i am wrong 🙏 )

  • eslint:recommended: https://github.com/eslint/eslint/blob/main/conf/eslint-recommended.js
  • @typescript-eslint/recommended: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/recommended.ts
  • qwik/recommended: https://github.com/BuilderIO/qwik/blob/main/packages/eslint-plugin-qwik/index.ts

can it be, that you use a global version of eslint in vscode which uses another config file? maybe we can get a step further when you do these steps on your app:

  • run pnpm lint (or npm / yarn) 😄
  • if the error is reported proceed, otherwise vscode uses another config
  • replace the lint script within the package.json file with "lint": "eslint src/**/*.ts* --debug"
  • search in the generated output for eslintrc:config-array-factory Loading JS config file and check if your root file is used
  • search also for consistent-type-imports

would be very interested to hear what the outcome of this is 🙏 thanks again for your help!

zanettin avatar Feb 17 '23 20:02 zanettin

@zanettin got your points.

harshmangalam avatar Feb 20 '23 03:02 harshmangalam

Hi @harshmangalam Just wanted to ask if you already had the time to check, where this rule is coming from 👼 Would be very interested to hear what the outcome is. Thanks again 🙏

zanettin avatar Feb 22 '23 21:02 zanettin

Hey I follow your process now everything is working fine.

harshmangalam avatar Feb 23 '23 03:02 harshmangalam

So glad to hear 🙏 Thanks anyways for creating this PR and doing the debugging! hope to see soooonish another PR from you 😃

zanettin avatar Feb 23 '23 11:02 zanettin