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

[2.5.0-beta] Cannot import typescript types if importsNotUsedAsValues is not "remove" #4581

Closed
7 tasks done
IanVS opened this issue Aug 11, 2021 · 11 comments
Closed
7 tasks done
Labels
bug: upstream Bug in a dependency of Vite

Comments

@IanVS
Copy link
Contributor

IanVS commented Aug 11, 2021

Describe the bug

The title is a little misleading, you cannot import types, but you can import type.

After #4279 began respecting tsconfig settings, I began getting errors from import statements which mixed value and types. I've narrowed it down to only happening when importsNotUsedAsValues is set to preserve or error.

So, for example, this has started to fail in my app:

import { useSelect, UseSelectProps } from 'downshift';

As a workaround, I can change it to:

import { useSelect } from 'downshift';
import type {UseSelectProps} from 'downshift';

But I think I should be able to mix them, right?

Note, this only happens in dev mode. Running yarn build && yarn serve does not throw an error in the console. Which makes me think this might be an esbuild bug, perhaps?

Reproduction

https://github.com/IanVS/vite/tree/ts-type-import

Clone my fork, checkout the ts-type-import branch, yarn, yarn build-vite, cd packages/playground/ts-import, yarn dev, and see the error in the browser console:

Uncaught SyntaxError: The requested module '/src/dep.ts' does not provide an export named 'Foo'

System Info

System:
    OS: macOS 11.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 1.07 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.4.0 - ~/.nvm/versions/node/v16.4.0/bin/node
    Yarn: 1.22.10 - ~/.nvm/versions/node/v16.4.0/bin/yarn
    npm: 7.19.0 - ~/.nvm/versions/node/v16.4.0/bin/npm
  Browsers:
    Brave Browser: 92.1.27.111
    Firefox: 90.0.2
    Safari: 14.1.1

Used Package Manager

npm

Logs

No response

Validations

@sodatea sodatea added bug: upstream Bug in a dependency of Vite and removed pending triage labels Aug 12, 2021
@sodatea
Copy link
Member

sodatea commented Aug 12, 2021

I think it's a bug in esbuild.

@swandir
Copy link
Contributor

swandir commented Aug 16, 2021

Reported the problem upstream evanw/esbuild#1525

@quolpr
Copy link

quolpr commented Sep 15, 2021

There is a response in related esbuild issue:

TypeScript might be changing their behavior for this soon. See microsoft/TypeScript#43393 and microsoft/TypeScript#44619. I think the new behavior will be something like --importsNotUsedAsValues=preserve still does not preserve imports that aren't used as values, but you can additionally add --preserveValueImports to actually preserve unused imported values. We'll have to see what the behavior ends up being when the next version of TypeScript is released. When that happens, esbuild can be changed to do what that version of TypeScript does.

So, how the current issue can be solved right now for vite? It adds some pain to the current dev experience

@IanVS
Copy link
Contributor Author

IanVS commented Sep 15, 2021

For now I've separated out my type imports from value imports. Honestly, I think it's cleaner and definitely more explicit that way anyway. But I agree it's not ideal to force devs to do that.

@swandir
Copy link
Contributor

swandir commented Sep 15, 2021

Another workaround is to set importsNotUsedAsValues to "remove"

@lgarron
Copy link
Contributor

lgarron commented Sep 16, 2021

I wanted to evaluate Vite for https://github.com/cubing/cubing.js/ and it mostly worked like a charm, except for this.

My VSCode tooling mixes type imports with value imports by default, and Vite's interpretation of "importsNotUsedAsValues": "error" doesn't seem to match TypeScript's.

Another workaround is to set importsNotUsedAsValues to "remove"

This is very helpful to know, thanks! I'd love for Vite to match TypeScript semantics for "importsNotUsedAsValues": "error" before we consider a full switch, though.

(This thread suggests that it requires a fix in esbuild, but we have been using that successfully to build the codebase for quite a while. I'm guessing Vite is passing something that esbuild isn't picking up on its own?)

@swandir
Copy link
Contributor

swandir commented Sep 16, 2021

(This thread suggests that it requires a fix in esbuild, but we have been using that successfully to build the codebase for quite a while. I'm guessing Vite is passing something that esbuild isn't picking up on its own?)

Yes, this change seems to have been introduced in #4279
Before that vite wasn't passing importsNotUsedAsValues to esbuild

@jonlepage
Copy link

For now I've separated out my type imports from value imports. Honestly, I think it's cleaner and definitely more explicit that way anyway. But I agree it's not ideal to force devs to do that.

The issu is if you use Vscode as IDE , it become really panful for the workflow

When you fix a type, if the type are inside a value module, it will mix with the import value instead create a new import type !
image

Actually this issu give more handwork because i always need check how vscode add some types and move some types inside new files.
Sometime you just want keep some types inside a value module because it logique to see there and that types there removes some ambiguity in his use from the value module.

I hope we can get a fix

FSMaxB added a commit to communityvi/communityvi that referenced this issue Sep 22, 2021
See vitejs/vite#4581
And evanw/esbuild#1525

This lead to npm run watch not working when upgrading svelte kit which in turn updated vite to 2.5
@dummdidumm
Copy link
Contributor

Version 0.14 of ESBuild contains a change in behavior with regards to this setting. The behavior of importsNotUsedAsValues is now aligned with that of TypeScript. To get back the previous behavior the new flag by TS is also supported now: preserveValueImports. The flag does what its name says, all value (non-type) imports are preserved regardless of whether they are used or not. Also see the TS 4.5 announcement: https://devblogs.microsoft.com/typescript/announcing-typescript-4-5/#preserve-value-imports

Therefore I think this issue can be closed.

@Niputi
Copy link
Contributor

Niputi commented Dec 17, 2021

@dummdidumm can be closed with vite 2.8 release with #5861 merged

@bluwy
Copy link
Member

bluwy commented Mar 14, 2022

Closing as #5861 is merged

@bluwy bluwy closed this as completed Mar 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug: upstream Bug in a dependency of Vite
Projects
None yet
Development

No branches or pull requests

9 participants