Skip to content
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

Hooks: Change UseState to be like react's use_state #129

Merged
merged 3 commits into from
Jan 27, 2022

Conversation

jkelleyrtp
Copy link
Member

@jkelleyrtp jkelleyrtp commented Jan 16, 2022

This PR modifies use_state to return (value, set_value) just like in React. This should make usestate easier to learn and easier to work with (direct matching, if/then/eq, etc).

More advanced usecases can use use_ref instead.

This is a very-much-breaking change - so the old UseState has been saved as "use_slot" with a deprecated tag, which we'll remove in the next release.

@jkelleyrtp jkelleyrtp changed the title Hooks: Change UseState to be cloneable into async contexts without for_async Hooks: Change UseState to be like react's use_state Jan 18, 2022
@jkelleyrtp jkelleyrtp marked this pull request as draft January 19, 2022 13:46
@jkelleyrtp jkelleyrtp marked this pull request as ready for review January 26, 2022 05:46
Copy link
Contributor

@t1m0t t1m0t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks neat and sound to me

@jkelleyrtp jkelleyrtp merged commit 75b1cff into master Jan 27, 2022
@jkelleyrtp jkelleyrtp deleted the jk/update-hooks branch January 28, 2022 03:19
@WestXu
Copy link

WestXu commented Feb 17, 2022

My 2 cents. I doubt if this is a good decision.

The previous use_state returns only one variable, with .set() to set its value, which is simpler and more intuitive. The react way looks more like a hack to me. We don't have to mimic everything from react.

@jkelleyrtp
Copy link
Member Author

My 2 cents. I doubt if this is a good decision.

The previous use_state returns only one variable, with .set() to set its value, which is simpler and more intuitive. The react way looks more like a hack to me. We don't have to mimic everything from react.

I feel like the most important question here is "messaging". Like, what should the signature of the hook say about how to use it?

The old "UseState" (the one-piece handle) is roughly the same with the new style as it was in the old style. It comes out as the second item in the tuple. We could even grab it out:

let name = use_state(&cx, || "bob").1;

match name.get() {
   "bob" => {},
   _ => {}
}

name.set("new_name");

There are a few challenges about these handles work in async, and the most reliable way of getting UseState working in async is to wrap it up behind a get/set interface.

Right now, the signature of use_state does not communicate that set_state has get/set/modify/make_mut etc. But, it does expose the value directly instead of keeping it locked behind a wrapper, which is much more intuitive than having .get or * all over your code to do simple thing.

@WestXu
Copy link

WestXu commented Feb 18, 2022

Oh right if that's the case. Thanks for the explaination.

jkelleyrtp added a commit that referenced this pull request Jun 28, 2023
Fix formatting issue with VSCode extension
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.

3 participants