immerjs / use-immer

Use immer to drive state with a React hooks
MIT License
4.04k stars 92 forks source link

Improve typings, updater will now return the new value instead of void #82

Closed omerman closed 3 years ago

omerman commented 3 years ago

Please tell me what you think, Im not sure that it is a good behavior.. :)

mweststrate commented 3 years ago

Hi @omerman, thanks for taking the effort of creating a PR!

For promises discussion, see #81. Slightly improved types have also landed as part of #78. Not sure if this PR is still relevant after that? Otherwise best rebase it first.

Ideally a PR should address only one item at a time (sadly Github can't stack them, that is a real omission), for traceability and reviewability :)

Thanks!

omerman commented 3 years ago

I think I'll close it.

The typings are cleared(Except for one minor thing, dropping the never with a more elegant typing, I'll open a small PR for that)

The idea of returning a promise that will be resolved with the new data could be a hassle and maybe should not be implemented as part of the hook, so I'll give it up