-
Notifications
You must be signed in to change notification settings - Fork 913
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
users get unfixable (and inaccurate) type errors when using go-to-source for openai library code #249
Comments
@RobertCraigie sure:
{
"extends": "../tsconfig.base.json",
"compilerOptions": {
"target": "es2022",
"lib": ["es2022"],
"types": ["@cloudflare/workers-types"],
"noUncheckedIndexedAccess": true
},
"include": ["src", "../common"]
}
{
"compilerOptions": {
"target": "es2022",
"module": "ESNext",
"moduleResolution": "Node",
"strict": true,
"allowSyntheticDefaultImports": true,
"noEmit": true,
"forceConsistentCasingInFileNames": true,
"resolveJsonModule": true,
"sourceMap": true,
"baseUrl": ".",
"paths": {
"@common/*": ["./common/*"]
}
}
} |
odd... although VSCode complains about these type errors, when i actually run very strange behavior. i'm using the latest version of VSCode and typescript 5.1. i should also mention that we do use other npm packages that use fetch in our server without issue. i wonder what is going on here... i've restarted VSCode a dozen times but keep getting the errors. if i remove the openai package, though, poof - no more type errors. |
Does it help to add |
Is there a relevant issue in VS Code for this? if not, perhaps worth opening one? |
Unfortunately not. Taking a step back... I can reproduce this issue in a blank project using version 4.3.0 of this library. Here's what I did:
{
"compilerOptions": {
"noEmit": true,
"target": "es2022",
"strict": true,
"moduleResolution": "Bundler"
},
"include": ["src"]
} EDIT: this is a red herring; see my comment below
|
ohhhh, i think the issue is that you're including the actual source |
Thanks @aroman , this is very helpful! I don't think the The tldr is that you need Is there a reason you want it off? |
Thanks @rattrayalex for the quick reply! I've done some more digging, and summarized my learnings below. Would love your thoughts when you get a chance! tl;dr
More detailed thoughts below to elaborate on the above. I've also re-titled the bug thread to better reflect what's going on here.
Ah, actually, that's my mistake — my To clarify what's actually happening here:
Fortunately,
|
.ts
files in the npm package, as users will get unfixable (and inaccurate) type errors when cmd+clicking on those files
Ah, terrific, thanks @aroman ! We'll look into whether there's a way to remove these errors - perhaps including a stub tsconfig could help. We'd like to keep the .ts files if possible because I believe it provides a much better go-to-definition experience, thanks to the declarationMap flag (which I wasn't aware of when I shipped the Stripe library's TypeScript support - I think it may not have been documented at the time). |
.ts
files in the npm package, as users will get unfixable (and inaccurate) type errors when cmd+clicking on those files
@rattrayalex yes, the go-to-definition is great! That's how I stumbled upon this in the first place 😄 And indeed, looks like a stub tsconfig does seem to do the trick. Dropping the below minimal tsconfig into {
"compilerOptions": {
"noEmit": true,
"target": "es2021",
"strict": true,
"moduleResolution": "Bundler"
},
"include": [ "src" ],
"exclude": [ "src/_shims" ]
} |
Ah, this is extremely helpful – thank you so much Avi! @jedwards1211 from our team will look to incorporate this tomorrow. |
@aroman I know you said it was a red herring, but I still can't repro following the steps in #249 (comment) using TypeScript 5.1.0 (and making sure VSCode uses that version). Would you mind trying to put together a repo that reproduces the issue? I worry that putting a Since afaik you have to use ESM for CloudFlare Workers, you should use It's hard to tell for sure but the errors in your OP seem to indicate TS wasn't successfully picking up the global types declared by |
@jedwards1211 try using the "go to definition" feature in that empty repo I suggested. the problem is not that it fails to compile (that's why I called it a "red herring"). the problem is that when you open the |
Alright sorry I was confused, I had been opening |
Facing same issue on my end, is there a temporary fix?
|
the error should go away if you close that file :) it shouldn't impact your code compiling at all. if you want a temporary fix for the error message, though, you can manually add the stub |
The fix for this will be out in the next release! |
This is breaking builds, hoping for a quick release of 4.5.0 🙂 |
Hmm, this still seems to be an issue in It seems like there is still no |
@robertherber anything breaking builds would be a separate github issue than this one, can you open a fresh issue (and include in it your (your quick fixes should be |
Confirm this is a library issue and not an underlying OpenAI API issue
Describe the bug
simply adding the new
openai
package to my project causes all sorts of type errors when I cmd+click on into the OpenAI source code from my projectTo Reproduce
See above.
OS
macOS
Node version
n/a
Library version
4.2.0
The text was updated successfully, but these errors were encountered: