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

export not found when executing typescript file #7384

Open
MRDGH2821 opened this issue Nov 30, 2023 · 12 comments · May be fixed by #16296
Open

export not found when executing typescript file #7384

MRDGH2821 opened this issue Nov 30, 2023 · 12 comments · May be fixed by #16296
Assignees
Labels
bug Something isn't working

Comments

@MRDGH2821
Copy link

What version of Bun is running?

1.0.14+d8be3e51b

What platform is your computer?

Linux 5.15.133.1-microsoft-standard-WSL2 x86_64 unknown

What steps can reproduce the bug?

  1. git clone https://github.com/MRDGH2821/Discord-Ban-Utils-Bot inside WSL.
  2. Switch to rewrite-sapphire branch
  3. Open the repo inside dev container
  4. bun src/index.ts

What is the expected behavior?

Code should run successfully

What do you see instead?

$ bun src/index.ts
56 |         }
57 |       }
58 |       return new FixedQueue;
59 |     }();
60 | 
61 |     function processTicksAndRejections() {
                                                                               ^
SyntaxError: export 'ValueOf' not found in './EventTypes'
      at processTicksAndRejections (:61:76)

EventTypes.ts is located in src/lib
And ValueOf is defined in src/lib/EventTypes.ts, re-exported in src/lib/utils.ts

Additional information

My bot requires the following to start:

  1. Discord bot token
  2. Firebase Cloud Firestore service account key file

And, I'm using Dev container inside WSL

@MRDGH2821 MRDGH2821 added the bug Something isn't working label Nov 30, 2023
@RobertP09
Copy link

I'm seeing this too. I have this import inside the the file where the error is occurring but bun doesn't recognize it..
Error:
SyntaxError: Export named 'Status' not found in module ***l/src/data/types.ts'. at processTicksAndRejections (:61:76)

import {
  STATUS,
} from '@src/types';

@fdaciuk
Copy link
Contributor

fdaciuk commented Dec 25, 2023

Same problem here with version 1.0.20:
image

I have a Context type exported from this file:
image

@nikhilro
Copy link

+1

export const OPENAI_MODEL_NAMES = ['gpt-4', 'gpt-3.5-turbo'] as const;
export type OpenAIModelName = (typeof OPENAI_MODEL_NAMES)[number];
SyntaxError: Export named 'OpenAIModelName' not found in module '/Users/nikhil/github/vapi/backend/types/openAITypes.ts'.
      at link (:1:21)
      at link (:1:21)
      at link (:1:21)
      at link (:1:21)
      at link (:1:21)
      at link (:1:21)
      at link (:1:21)
      at linkAndEvaluateModule (:1:21)
      at processTicksAndRejections (:61:77)
FAIL: 1

@Jarred-Sumner
Copy link
Collaborator

TypeScript types don't exist at runtime, so importing them doesn't work.

When you want to import types from files, you must tell Bun that it is a TypeScript type and not a file:

import {type OpenAIModelName} from '...'

Instead of

import {OpenAIModelName} from '...'

Or you can do

import type {OpenAIModelName} from '...'`

Please set verbatimModuleSyntax: true in tsconfig.json, so that TypeScript helps you more with this issue.

I'm closing this as "Not planned" because the fix is to specify type before the import name and the error itself is correct ("OpenAIModelName", ValueOf, etc are not JavaScript exports, they're TypeScript types which do not otherwise exist at runtime)

Please leave a comment if that explanation isn't enough to help you fix this

@Jarred-Sumner Jarred-Sumner closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2023
@MRDGH2821
Copy link
Author

MRDGH2821 commented Jan 27, 2024

Thanks for the solution!

Today, I also found an alternative for those using eslint & don't want to enable verbatimModuleSyntax in tsconfig.json

Adding this into the config & using eslint --fix takes care of the imports.

{
  "rules": {
    "@typescript-eslint/consistent-type-imports": "error",
    }
}

Requires typescript-eslint stuff to work.

@dougludlow
Copy link

dougludlow commented Apr 7, 2024

This does make it much harder to migrate an existing project to bun. We have to try and go through each of the thousands of files in our project and add the type modifier. Unfortunately, the error in bun doesn't point to the exact line where the import is at so it's hard to find sometimes.

@dougludlow
Copy link

vscode automatically adds imports without adding the type modifier, there are too many files to edit to make using bun viable for our existing project...

@dougludlow
Copy link

dougludlow commented Apr 7, 2024

Well, I went the nuclear option and ran eslint --fix after adding the above mentioned rule. We're using nx, so I ran bunx nx run-many --target lint --fix. I just have 1K+ files to commit...

Edit: Only that didn't actually work because the eslint rule is too strict. We're using NestJS and now nest doesn't recognize the types that are being injected due to the type modifier that the eslint rule automatically added... 🤦

I need a tool that's a bit smarter than this eslint rule that actually evaluates the type original type that's being imported to tell if it's type or interface and then adds the modifier...

Edit 2:

I found that if you run eslint with type-aware linting it will ignore instance of types used in decorators and constructors, which gets me a lot closer to where I can run the application, but not quite. There are still quite a few places I need to manually add the type modifier, but it's a little more manageable.

Also, for vscode/typescript 5.3 there is an option to prefer type imports: typescript.preferences.preferTypeOnlyAutoImports

@rafsawicki
Copy link

rafsawicki commented Apr 30, 2024

This issue is definitely hurting adoption in our company for backend projects. The expectation is typically that if project was previously ran through ts-node in development, it should be able to run after replacing it with bun run. Right now is fails more often than not, because well, no one is bothering with using import type if regular imports work perfectly fine without bun, both in development and after compilation to JS.

It kinda looks like even though bun is saying that is it supports Typescript, it more like it supports just a subset of it, and that using ESLint is mandatory to ensure that there are no runtime errors when using unsupported features. In my opinion, if issues like this are closed as "not planned", the project documentation should mention that it will never suport all features, and should recommend a specific ESLint configuration to not accidentally run into runtime errors with perfectly valid Typescript code.

@collettemathieu
Copy link

I had the same problem with bun test concerning types. When bun runs the tests, it is in an already compiled environment, so it doesn't logically find the types.

For me, the solution was to specify not only when importing but also when exporting types.

Example:

// I import the type from the library
import type { myType } from '@project/my-type-lib'; 
// I export the type from the library
export type { myType } from './lib/my-type.types';

I didn't need to configure ESLint. In fact, on this project I was using biome.

On the contrary, I think it's a good thing to have to type imports/exports properly.

thejetou pushed a commit to smogon/damage-calc that referenced this issue Aug 8, 2024
I'm mainly interested in getting this merged to eventually run `damage-calc` under bun (they apparently don't support importing types without doing this oven-sh/bun#7384 (comment))

The changes are mostly generated with the new eslint rule + `eslint --fix .` (with some manual fixes after)

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
@brianmhunt
Copy link

brianmhunt commented Nov 15, 2024

Running the tests in Jest works, running bun test is emitting errors like this:

Image

I think for significant projects (ones closing on 1m+ lines, thousands of files/tests), this is practically unfixable.

A better error message would make a transition more achievable, specifically the file where the import is failing. Any context at all would be an improvement.

Alternatively bun compatibility with jest would help, though we'd just as soon fix these sorts of issues.

@Jarred-Sumner Jarred-Sumner reopened this Dec 7, 2024
@Jarred-Sumner
Copy link
Collaborator

Jarred-Sumner commented Dec 7, 2024

We are going to fix this. It will involve changes to JavaScriptCore (the engine) to better support TypeScript.

Here's roughly how we're planning to fix it:

Step 1

  1. When Bun transpiles a file, we add a new SourceProviderSourceType::TranpsiledModule https://github.com/oven-sh/WebKit/blob/main/Source/JavaScriptCore/runtime/JSModuleLoader.cpp#L351-L381
  2. For TranspiledModule, we replace the work of ModuleAnalyzer to instead read directly from Bun's AST, using the import records and export records similarly: https://github.com/oven-sh/WebKit/blob/main/Source/JavaScriptCore/runtime/JSModuleLoader.cpp#L383-L390
  3. PR this by itself.

This first step does two things:

a) Remove redundant parsing step, improving ESM performance a little bit.
b) will also unblock letting us bytecode cache ESModules. Currently, there is zero performance gain from bytecode caching ESModules because it still re-parses the whole file due to the ModuleAnalyzer. Integrating that into bun build --bytecode can be done as another PR, where we serialize the import_records and export records from the AST and make that work somehow

Step 2

  1. We add TypeScript to ExportEntry::Type and Resolution::Type. Then, we make it an export which will never hit the ::ambiguous() case which means we don't need to worry about ambiguous re-exports from TypeScript types.
  2. We make it so TypeScript type imports never collide with runtime types. The analyzed module (from step 1) would add the typescript-only types and that's how we would propagate them across the module graph at runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants