data of entity.delete().go() should be nullable

tywalch / electrodb

A DynamoDB library to ease the use of modeling complex hierarchical relationships and implementing a Single Table Design while keeping your query code readable.

MIT License

1.03k stars 66 forks source link

data of entity.delete().go() should be nullable #431

Closed ikeyan closed 1 month ago

ikeyan commented 1 month ago

Delete method doesn't have ConditionExpression that the item being deleted exists by default.

fix: change the data type from Partial<T> to Keys | null when response option is absent or undefined
fix: make the data types nullable in all cases
refactor: remove O["response"] extends "all_new" case because O extends DeleteQueryOptions and DeleteQueryOptions['response']?: "default" | "none" | "all_old" does not include "all_new"

netlify[bot] commented 1 month ago

Deploy Preview for electrodb-dev ready!

Name	Link
Latest commit	7917f85c30a5ae51d972f415534f5a6718f05931
Latest deploy log	https://app.netlify.com/sites/electrodb-dev/deploys/66fe498855dd240008970ac8
Deploy Preview	https://deploy-preview-431--electrodb-dev.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

zirkelc commented 1 month ago

I'm not entirely sure, but I think it's on purpose that Entity.delete() doesn't add any conditional check in order to be close the DynamoDB DeleteItem operation. There is the convenience method Entity.remove() which adds the conditional check.

ikeyan commented 1 month ago

@zirkelc yes, Entity.delete() have no problem in implementation. I just changed its typing.

zirkelc commented 1 month ago

Sorry, I should have read more carefully 😄

tywalch / electrodb

data of entity.delete().go() should be nullable #431

✅ Deploy Preview for electrodb-dev ready!

Deploy Preview for electrodb-dev ready!