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

Vscode not import types and values declaration correctly when they are in same file #136769

Closed
jonlepage opened this issue Nov 9, 2021 · 5 comments
Assignees
Labels
*as-designed Described behavior is as designed

Comments

@jonlepage
Copy link

jonlepage commented Nov 9, 2021

Does this issue occur when all extensions are disabled?: yes

  • VS Code Version:
    Version : 1.62.0 (user setup)
    Commit : b3318bc
    Date : 2021-11-03T15:23:01.379Z
    Electron : 13.5.1
    Chrome : 91.0.4472.164
    Node.js : 14.16.0
    V8 : 9.1.269.39-electron.0
    OS : Windows_NT x64 10.0.19042

Steps to Reproduce:

  1. create 2 file , examle A.ts and B.ts
  2. add in file A.ts type and concret variable
  3. try import in B.ts , A.ts stuff, and let vscode help you to import stuff, it will not fellow rule correctly.

Hi everyone, is it possible to maybe add a option to separate type and value import correctly, because this give big head hash if we use a ts rule like:
"importsNotUsedAsValues": "error",

Example if we have 2 file without import:
test1.ts

export type helperA = any;
export const A = "Concret";

test2.ts

export const b:helperA = {};
const x:string = A

And we mouse hover const x:string = A;
Vscode will suggest fix to import value
image
And vscode will write for you the import, this is perfect and super productive !

result:import { A } from './test1';


After if we mouse over the second declaration
Vscode will suggest import as value in same import declaration
image

result:import { A, helperA } from './test1';

This is maybe correct but is not what we want if we use rule like
"importsNotUsedAsValues": "error",


Vscode should understand this rule and suggest **add this as type separately **
Expected result:

import { A } from './test1';
import type { helperA } from './test1';

Because actually import type and value in the same import make broken some bundler like vitejs.
Error like
Uncaught SyntaxError: The requested module '/src/core/test1.ts?t=1636477934311' does not provide an export named 'Interpreters'
So i need to always manually change the import made by vscode, where is little waste of time !
or change my rule by "importsNotUsedAsValues": "remove" where is not what i want !

Here related discutions that's explain this new issue.
evanw/esbuild#1525
microsoft/TypeScript#43393

So to make everything fine, i think if we can maybe add a setting in vscode to say Prefer import type and value separately it will be nice

No more need to hack manually what Vscode suggest when import stuff !
I say add a new option, if for you, because i dont know this can affect some other pattern of project, but on my side i want keep error, and allowed to import type and concret from same file
(i know it recommend to separate types and values, but it recommend, is not a law !).
thanks

@mjbvz
Copy link
Contributor

mjbvz commented Nov 9, 2021

Does this reproduce in the latest VS Code insiders build with TS 4.5+?

@mjbvz mjbvz added the info-needed Issue requires more information from poster label Nov 9, 2021
@jonlepage
Copy link
Author

Does this reproduce in the latest VS Code insiders build with TS 4.5+?

yep,
image
if the last vscode is 1.62.0

@mjbvz
Copy link
Contributor

mjbvz commented Nov 9, 2021

The current insiders version is 1.63. Please test using it and also make sure the workspace version of TS is 4.5+ (not just tsc)

@jonlepage
Copy link
Author

jonlepage commented Nov 12, 2021

The current insiders version is 1.63. Please test using it and also make sure the workspace version of TS is 4.5+ (not just tsc)

@mjbvz sry ! , it taked me time to have a free moment.

I installed Insider and confirm the insider have same behavior with the rule "importsNotUsedAsValues": "error",
Vscode always merge types in values in the same import, and will make crash the new and last EsBuild version.
Tested on

Version: 1.63.0-insider (user setup)
Commit: c1096372119bcf83b9bd8ccee96dcf266f35de76
Date: 2021-11-11T13:18:41.690Z
Electron: 13.5.2
Chromium: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Windows_NT x64 10.0.19042

and seem ts 4.5.1-rc

image

image

I take time to make you a little gif also if it can help to understand what i mean with my lack of English 😁
So when we separate declaration: "type and value", Esbuild success to remove the import type, but no when they are in the same declarations. "they will maybe add a fix or support one day but this is not the point"

vscode import as type separatly
Hope those info help, thanks for all your awesome work !


NOTE: also we should not forget new ts version will allow type tag inside the same import
https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-rc/#type-on-import-names

So this will be also valide in next release: import {A, type helperA} from 'test';
Maybe also take in account both patterns for vscode auto suggest, at the taste of the user with a setting !?

// valide pattern 1

import {A} from 'test';
import type { helperA} from 'test';

// valide pattern 2

import {A, type helperA} from 'test';

@mjbvz
Copy link
Contributor

mjbvz commented Nov 19, 2021

Thanks for testing. As you note, the way the import is added is fine — and seems preferable in most cases to having two imports for the same file. We generally don't adapt our generic TS behavior to very specific eslint rules like this unless we get significant feedback (and the existing behavior has been in place for a while)

Therefore I'm closing this as by-design

@mjbvz mjbvz closed this as completed Nov 19, 2021
@mjbvz mjbvz added *as-designed Described behavior is as designed and removed info-needed Issue requires more information from poster labels Nov 19, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed
Projects
None yet
Development

No branches or pull requests

2 participants