xplato / useUndoable

↪ React hook for undo/redo functionality (with batteries included)
MIT License
168 stars 7 forks source link

undoable when using setValues with context provide old state in function #20

Closed geril2207 closed 1 year ago

geril2207 commented 1 year ago

useUndoable always provide new setter of state (second prop of array) and because of that older setter when using setValues(prev => ({prev}) get a old prev state. When using useState prev argument always will be actual state and setState will be the same function. I have an issue when my app drop some fields from state because of that.

Example: There is interval that set value. Type smth in inputs, these values will be drop, because in prev state this values will be old because prevState depends on setValues version

If remove setValues from useEffect deps prev will be old state, if u change useUndoable to useState the issue was gone. But if you add setValues in useEffect deps it also was fine. I think useUndoable should be same by API with useState

https://codesandbox.io/s/floral-moon-d4ycmi

geril2207 commented 1 year ago

I fix my issue by passing setValues into useRef, maybe do smth like this under the hood?

geril2207 commented 1 year ago

Why not pass state into ref? image image

xplato commented 1 year ago

Hey @geril2207, thanks for opening this! Thank you for your patience here. I will be looking into this issue this week!

xplato commented 1 year ago

@geril2207 I'm afraid I'm not entirely sure what the issue is here. What do you mean by "drop values?"

Also, have you looked into the static_setState function? By design, it's not a functional updater, so it doesn't have to change along with your state. Could you more clearly describe what is happening and what the expected behavior is?

Your CodeSandbox is a little reductive; I'm not sure if an interval is the proper method of checking the value over time...

geril2207 commented 1 year ago

@xplato by static_setState you can't get prev snapshot of state(using function with prev => {} to update state). With setValues u can do this but there prevState actually does not provide actual state(only actual state for current link of function). Yeah i agree with you about my code sandbox, it doesn't show anything. But what if you have two setValues in one function(for example, in real apps you can have handlers that executes with little delay useEffects, other triggers, and in this case you will have an old link to setValues and not actual prevState.

I think about this cases image image

In this cases second setValues will be not actual setValues and therefore will have an old prevState. To have an actual prevState you always need to have actual setValues link on function.

I hope you understood me)

geril2207 commented 1 year ago

I think it's easily fixes with passing setValues into ref or state inside of hook. Because if you pass state into ref it does not matter which actual or not actual link on setValues fn you have, you always provide actual prevState like useState API. But if nobody in community worries about this, should let developers option to manually pass setValues into ref to always have an actual link.

xplato commented 1 year ago

Ah, I understand now. I think this is a natural limitation of state updates with useState, no? You are correct that passing this into a ref would give direct access to the current state, though I am hesitant to make this change because it might introduce very subtle state update bugs with projects expecting the more synchronous method of access.

geril2207 commented 1 year ago

@xplato in useState does not matter which link on setState you use old or new, in all cases your prevState will be actual snapshot of state. In example i have two setValues in one function. I think you can avoid this by batching updates into one setValues, but in my case i have one trigger that execute first update in one place, and then execute second update in another place(different components). In this case component does not render and second update executes on older link on setValues and therefore i have no first updates in second prevState.Yeah, this change maybe can provide some bugs.