search

typesense / typesense-js

JavaScript / TypeScript client for Typesense
https://typesense.org/docs/api
Apache License 2.0
393 stars 74 forks source link

refactor: improve typing as required by tsc 5.3 or higher #186

Open berenddeboer opened 8 months ago

berenddeboer commented 8 months ago

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 commented 8 months ago

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

jasonbosco commented 8 months ago

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

berenddeboer commented 8 months ago

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.

jasonbosco commented 8 months ago

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

berenddeboer commented 8 months ago

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.

MikeAlexMartinez commented 5 months ago

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

berenddeboer commented 5 months ago

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

MikeAlexMartinez commented 5 months ago

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.