typesense-js icon indicating copy to clipboard operation
typesense-js copied to clipboard

refactor: improve typing as required by tsc 5.3 or higher

Open berenddeboer opened this issue 2 years ago • 8 comments

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

berenddeboer avatar Dec 11 '23 19:12 berenddeboer

See also #181 which mentions ImportError as well as an issue.

berenddeboer avatar Dec 11 '23 19:12 berenddeboer

Thank you for the PR @berenddeboer. This looks mostly fine to me. Could you address the issues reported in the CI pipeline?

jasonbosco avatar Dec 12 '23 20:12 jasonbosco

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.

berenddeboer avatar Dec 13 '23 20:12 berenddeboer

Would upgrading the Typescript package in package.json to 5.3 help?

jasonbosco avatar Dec 13 '23 20:12 jasonbosco

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.

berenddeboer avatar Dec 15 '23 22:12 berenddeboer

Is this still being worked on? I'm encountering the same issues and would like to fix this if possible

MikeAlexMartinez avatar Mar 14 '24 12:03 MikeAlexMartinez

Up to @jasonbosco to decide if he wants to allow an implicit any or not. I fixed the issue at that time.

berenddeboer avatar Mar 14 '24 19:03 berenddeboer

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.

MikeAlexMartinez avatar Mar 15 '24 13:03 MikeAlexMartinez