blinkdb-js / blinkdb

🗃️ An in-memory JS database optimized for large scale storage on the frontend.
https://blinkdb.io
MIT License
119 stars 10 forks source link

Avoid type assertions in update.ts #6

Closed grumd closed 2 years ago

grumd commented 2 years ago

Hi.

I've looked at your source code and noticed that you're heavily using type assertions, as well as such code smells as "as any".

I want to use this pull request as an encouragement and example to show that type hacks are unnecessary in your codebase and you can do better. If you want to call your library "type-safe" please make sure your internal code is type-safe first and foremost.

Unfortunately I don't have enough time to fully refactor all of your code, so only took liberty of doing it in a couple of files as a proof of concept.

Cheers.

netlify[bot] commented 2 years ago

Deploy Preview for blinkdb canceled.

Name Link
Latest commit 0aa06fccadb561887b410f766cb6e44acdc9ea0b
Latest deploy log https://app.netlify.com/sites/blinkdb/deploys/6323a44a62fea60009e5e379
grumd commented 2 years ago

Note: you may want to change your usages of <T> to <T extends object> for the entity type. Doing keyof T when T is not always an object can be troublesome from the type safety standpoint.

maradotwebp commented 2 years ago

Thanks a lot for your contribution :)

Developing BlinkDB, I wanted to focus on type-safety for the user first. I know that internally, the code isn't as type-safe as it could be, but I hope to remedy that in time.