refactor: improve typing as required by tsc 5.3 or higher#186
refactor: improve typing as required by tsc 5.3 or higher#186berenddeboer wants to merge 2 commits intotypesense:masterfrom
Conversation
|
See also #181 which mentions ImportError as well as an issue. |
|
Thank you for the PR @berenddeboer. This looks mostly fine to me. Could you address the issues reported in the CI pipeline? |
|
Thanks @jasonbosco , unfortunately addressing the pipeline issues means removing the ts directive, which would make the library fail in newer typescript versions. The reason it reports this as unnecessary is because this is an older tsc. |
|
Would upgrading the Typescript package in package.json to 5.3 help? |
|
Would have sworn yes, but you are are right. It's the implicit any on tsconfig.json. Have forbidden this now, and now the error disappears. Not sure what you think about it, but imo disabling this forces people to think about types more. |
|
Is this still being worked on? I'm encountering the same issues and would like to fix this if possible |
|
Up to @jasonbosco to decide if he wants to allow an implicit any or not. I fixed the issue at that time. |
|
I managed to fix my issues. I was importing the types from the src directory instead of the lib so it was reading lots checking the typesense project more than necessary. |
Change Summary
The current library generates about 20 errors when included in a typescript project, possibly only when using TypeScript 5.3 or higher, see #183.
This is my first PR, a bit unclear about the process. For example "npm run format" formats a whole bunch of things, not related to my PR. So just stuck to what I guessed to be the code style in this library.
"npm build" generates errors about unused suppressions, not sure what the fix should be there. The suppressions are clearly needed, but the code could be improved to be more typesafe.
I'm also unsure if ImportError works correctly, it gets passed in two very different types, and it looks to me the array version is an error, but this is what the current code does.
PR Checklist