-
Notifications
You must be signed in to change notification settings - Fork 38
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
[Bug] v1.0.0 transpile errors #102
Comments
Thanks for filing, I never saw this error while developing with typescript in our various sample apps. I need to do some investigation to understand what's going wrong, but if typebox ends up being problematic we can refactor away from it. |
Thanks for the response @jhamon. If it helps, here is the tsconfig json we use:
And our project dependencies:
|
@jhamon In my firebase code I am also having an issue: tsjson:
project dependencies:
|
@jhamon Repro if needed:
|
Thanks everyone for the details. Will follow-up soon with a fix. |
## Problem Several users have reported issues with typescript compilation This PR addresses: - #102 - #105 ## Solution ### Clean up unused dependencies to resolve web-related type errors I started by deleting dependencies and running tests to see what dependencies were no longer in use. I was able to delete these: - @types/web - @jest/globals - unique-names-generator Removing `@types/web` seems to have resolved the majority of Typescript issues related to conflicts with web types in `lib.dom.d.ts`. Since I was already in there mucking with dependencies, I went ahead and shuffled a few things between dependencies/devDependencies just to align with my current understanding of things. ### TS1005 (Typescript versions <=4.4) People on older typescript versions report seeing many errors like this: ``` Error: ../pinecone-ts-client/dist/control/index.d.ts(1,31): error TS1005: ',' expected. ``` These errors were caused by using import statements that combined type and value imports onto one line. This syntax was evidently not supported in versions of typescript prior to 4.4. Resolving these issues involved refactoring about 30 import statements along these lines: ```diff -import { IndexNameSchema, type IndexName } from './types'; +import { IndexNameSchema } from './types'; +import type { IndexName } from './types'; ``` ### Typebox errors (Typescript >= 5.1) The `@sinclair/typebox` dependency is causing issues with very recent versions of typescript. ``` ../node_modules/@sinclair/typebox/typebox.d.ts:179:167 - error TS2589: Type instantiation is excessively deep and possibly infinite. 179 ] extends [TArray, TNumber] ? AssertType<T['items']> : K extends TTemplateLiteral ? TIndexReduce<T, TTemplateLiteralKeyRest<K>> : K extends TUnion<TLiteral<Key>[]> ? TIndexReduce<T, TUnionLiteralKeyRest<K>> : K extends TLiteral<Key> ? TIndexReduce<T, [K['const']]> : TNever; ``` ### New CI job: `typescript-compilation-tests` To verify this compilation issue doesn't come up again, I added a new github actions job along with a version matrix. It tests every major version of typescript back to version 4.1 (released [fall 2020](https://devblogs.microsoft.com/typescript/announcing-typescript-4-1/)) along with a version of pinecone, then checks whether it can successfully `tsc`. I ran these with the v1.0.0 pinecone release to see the jobs fail as reported, then updated it to install the latest code in this branch and saw them pass. This will be part of CI for every PR from now on. ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) - [x] This change requires a documentation update - [x] Infrastructure change (CI configs, etc) ## Test Plan Should see new compilation tests passing in CI. There are now 13 different checks running in CI to exercise compilation with major versions of TypeScript. ## Follow-up Probably we should add a lint rule to prevent us adding back any mixed type/value imports.
I just published the Given the number of moving parts involved with Typescript it's still possible there are undetected issues. We'll have to continue expanding our compilation testing to cover these cases as they pop up. |
@jhamon Compiles perfectly, big thanks. |
I'm going to close this issue as I believe a wide variety of common issues were resolved in PR #114 that shipped in |
Perfect. |
Is this a new bug?
Current Behavior
When running
npx tsc
several errors occur.Expected Behavior
Should transpile correctly.
Steps To Reproduce
Relevant log output
Environment
Additional Context
No response
The text was updated successfully, but these errors were encountered: