use_state setter causes re-render when same value is set

[voidpumpkin]

Problem
Yew use_state setter causes re-render when same value is set

Steps To Reproduce
Use a use_effect that sets None value to use_state setter when the value already is None

Here is a small repo with the problem:
https://github.com/voidpumpkin/yew-playground/tree/use-state-setter-compare-problem

Expected behavior
Not to re-render

Screenshots
Not the problem but the representation of the example project already running:

Guess of how to fix
I think use_state type T would have to implement PartialEq trait.
Then before creating a new pointer to "new" value, it first would check if the value actually changed.
I believe i can try to fix this, just let me know there would be a green light for it.

Environment:

• Yew version: [master]
• Rust version: [ 1.55.0,]
• Target, if relevant: -
• Build tool, if relevant: -
• OS, if relevant: -
• Browser and version, if relevant:-

Questionnaire

• I'm interested in fixing this myself but don't know where to start
• I would like to fix and I have a solution
• I don't have time to fix this right now, but maybe later

[mc1098]

This has been discussed previously in Discord, but the TLDR is that it might be useful to have this behaviour of re-rendering regardless of whether the value is the same but I think it would also be nice to have another method which is available when T: PartialEq. I think the only reason this hasn't happened yet is because of naming 🙃.

To further the point of wanting this behaviour, you can see some of the discussion with @yoroshikun and myself here regarding #2088. This behaviour also allows for using use_state with () in order to force a render, which might be useful when you are using use_ref to avoid cloning a large struct like would have to if you used use_state on it's own.

Just seen your PR as I was typing this - sorry I was late to reply before you got stuck in 🙏

[voidpumpkin]

@mc1098 No problem about the PR, i just did it quickly without much though.

I had a feeling this is intentional.

I come from react.js world, there actually this kind of functionality was default (to not re-render).

What would you think if there were two hooks?

• use_state that does comparison automatically
• use_state_no_compare that does not compare

[voidpumpkin]

O even better two setters in UseStateHandle
set that checks the coparison and set_no_checks that does not

[mc1098]

O even better two setters in UseStateHandle set that checks the coparison and set_no_checks that does not

I don't even think you need the extra setter, I think because the value is available you can do the check at the function call and just not call the setter if the equality check is true for set.

I come from react.js world, there actually this kind of functionality was default (to not re-render).

I don't have a strong feeling about whether the default should be checked or unchecked - but I think if we do it through the impl of UseStateHandle then the current behaviour makes sense as the default because the function would always be available whereas the checked version would only be available when T: PartialEq.

[yoroshikun]

I come from react.js world, there actually this kind of functionality was default (to not re-render).

Just my two cents, I also come from the react.js world and I often found it more readable to handle part equal outside the setter, this removes complexity of the hook and offers a clearer syntax to when a render should happen.

In the case of yew I am on the side of no checking at all by default, while an extended custom hook could serve your needs especially for custom types that will need to implement PartialEq.

TLDR: I find the workaround of manual checking before setting is the clearer option here. And will not force a partialEq type to be required

[mc1098]

In the case of yew I am on the side of no checking at all by default, while an extended custom hook could serve your needs especially for custom types that will need to implement PartialEq.

So I think UseStateHandle should have this equality check set fn, you can optionally avoid it if you prefer checking outside. I think it would be easy to add and makes sense that use_state can have this choice available. I think a custom hook for this would be 99% use_state with a small twist which irks me 🙃

impl<T: PartialEq> UseStateHandle<T> {
    pub fn set_not_eq(&self, value: T) {
        if *self.value != value {
            (self.setter)(value);
            true
        } else {
            false
        }
    }
}

// doesn't impl or derive PartialEq
struct Demo;

let demo = use_state(|| Demo);
// set is always available
demo.set(Demo);
// set_not_eq won't be usable because Demo doesn't satisfy the trait bound `Demo: PartialEq`
demo.set_not_eq(Demo);

And will not force a partialEq type to be required

Notice that implemented like this means you can still use use_state even if T does not have the PartialEq bound :)

Probably want a better name than set_not_eq though 🙃,

@Madoshakalaka and @WorldSEnder were both discussing this in Discord so would be interesting to hear from them whether they had thought about this more :) btw hope you two don't mind the ping 😨

[voidpumpkin]

Opened a PR for the discussed change.