Skip to content

Provide a stable setState method #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stefanhayden
Copy link

Hello. Thanks for all the hard work you put in here.

I was a bit surprised the setState function was not stable. In react when using useState that function is always stable. This repo, which does try and mirror that pattern, has made a different choice. I was looking at past conversations and I don't 100% understand why, but no worries.

I was wondering if you could also export a stable function like the one I have in this PR? I'm sure the static_setState is helpful to some but I don't see a reason to not also provide a more common stable function. I've been using it in my own project and I don't know of any downsides.

again, thanks for your hard work and I hope this kind of addition is considered.

@xplato
Copy link
Owner

xplato commented Oct 8, 2024

Hey @stefanhayden, thanks for opening a PR!

The lack of stability in the setState function was an unfortunate mistake I made in some of the initial versions of useUndoable. It was, in fact, something I wanted to address in version 6, which I have a draft PR for now (although, I haven't been able to get back to it for some time).

Since v6 is a significant refactor, I would want to include these changes there (more precisely, I'll be replacing the definition of setState with the one you have here). I'll keep this PR open until I get those changes integrated, and give you an update at that time.

Thank you again for taking the time to improve the code here! You rock!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants