Is it safe to ignore some properties from PartialEq for a struct component.

[zstewar1]

I have a Struct Component which switches itself between displaying text as a normal div/span and being an input text box when clicked. The parent controls the final text when the edit is committed, but the component controls its stated during editing. This isn't quite a minimal example, but I've tried to cut it down to the most relevant bits:

#[derive(Debug, Properties)]
pub struct Props {
    /// Last committed value.
    pub value: AttrValue,

    // ...
    // ... Other properites which also affect the output of view
    // ...

    /// Commit an updated value.
    pub on_commit: Callback<AttrValue>,
}

pub enum Msg {
    /// Message during editing to update the edited text.
    UpdateInput { input: AttrValue },
    /// Message while not editing to start editing.
    StartEdit,
    /// Message to finish editing.
    FinishEdit,
    /// Cancel editing without changing the value.
    Cancel,
}

/// Helper to display some text with click-to-edit.
pub struct ClickEdit {
    /// Pending edit text if clock speed is being changed.
    edit_text: Option<AttrValue>,
    /// Whether we did focus since last committing an edit.
    did_focus: bool,
    /// Input to focus on editing.
    input: NodeRef,

    // Memoized callbacks:
    oninput: Callback<InputEvent>,
    onkeyup: Callback<KeyboardEvent>,
    onblur: Callback<FocusEvent>,
    onsubmit: Callback<SubmitEvent>,
    onclick: Callback<MouseEvent>,
}

impl Component for ClickEdit {
    type Message = Msg;
    type Properties = Props;

    fn create(ctx: &Context<Self>) -> Self {
        let link = ctx.link();
        ClickEdit {
            edit_text: None,
            did_focus: true,
            input: NodeRef::default(),

            oninput: link.callback(|input| Msg::UpdateInput {
                input: get_value_from_input_event(input),
            }),
            onkeyup: link.batch_callback(|e: KeyboardEvent| match &*e.key() {
                "Esc" | "Escape" => Some(Msg::Cancel),
                _ => None,
            }),
            onblur: link.callback(|_| Msg::FinishEdit),
            onsubmit: link.callback(|e: SubmitEvent| {
                e.prevent_default();
                Msg::FinishEdit
            }),
            onclick: link.callback(|_| Msg::StartEdit),
        }
    }

    fn update(&mut self, ctx: &Context<Self>, msg: Self::Message) -> bool {
        match msg {
            Msg::UpdateInput { input } => {
                self.edit_text = Some(input);
                true
            }
            Msg::StartEdit => {
                self.edit_text = Some(ctx.props().value.clone());
                self.did_focus = false;
                true
            }
            Msg::FinishEdit => {
                if let Some(edit_text) = self.edit_text.take() {
                    ctx.props().on_commit.emit(edit_text);
                    true
                }
            }
            Msg::Cancel => {
                self.edit_text = None;
                true
            }
        }
    }

    fn view(&self, ctx: &Context<Self>) -> Html {
        let Props {
            value,
            ..
        } = ctx.props();
        let class = self.class.clone();
        if let Some(value) = self.edit_text.clone() {
            let oninput = self.oninput.clone();
            let onkeyup = self.onkeyup.clone();
            let onblur = self.onblur.clone();
            let onsubmit = self.onsubmit.clone();
            html! {
                <form {onsubmit}>
                    <input type="text" value={&value}
                        {oninput} {onblur} {onkeyup} ref={&self.input} />
                </form>
            }
        } else {
            let onclick = self.onclick.clone();
            html! {
                <div {onclick}>{value}</div>
            }
        }
    }

    fn rendered(&mut self, _ctx: &Context<Self>, _first_render: bool) {
        if !self.did_focus {
            if let Some(input) = self.input.cast::<HtmlInputElement>() {
                if let Err(e) = input.focus() {
                    warn!("Failed to focus input: {:?}", e);
                }
                input.select();
                self.did_focus = true;
            }
        }
    }
}

What's important here is that the on_commit callback is only ever used from update; it has no effect on view as it is not attached to any descendant element directly. My question is, would it be safe to custom-impl PartialEq for the props of this component to exclude on_commit from it?

My concern is that I don't know if Yew will skip updating entirely if two old props and new props compare equal, or if it will attach the new props to the context but skip calling view (so the next time there is a call to update, update will have the most recent props.

I could of course implement custom comparison logic in changed, but that would presumably duplicate work that Yew already does; I need to re-draw any time any property besides on_commit changes, so I would have to compare all of the fields besides on_commit, which Yew already does before calling changed.

So I just want to know if Yew guarantees that a struct component's Context includes the most up to date properties, even if those properties compared equal to the old properties according to PartialEq.

[WorldSEnder]

Yew compares with the existing props and only replaces if they compare unequal. In this case, changed is then called to figure out if the component needs to re-render. Note that this callback also gives you access to the old props. So the correct way to avoid a re-render is to implement changed here.

Small comment about the concern about duplicating work: The relevant code section is monomorphized per component, so I would think that the compiler is smart enough to inline changed in most cases and remove the duplicate checks in a lot of cases.

Relevant code:

yew/packages/yew/src/html/component/lifecycle.rs

Lines 211 to 216 in 2e464ed

	if self.context.props != props {
	let old_props = std::mem::replace(&mut self.context.props, props);
	self.component.changed(&self.context, &old_props)
	} else {
	false
	}

[zstewar1]

That is helpful, thank you.

Out of curiosity, why does yew preemptively check equality first, rather than always delegating to changed?

[WorldSEnder]

I think the decision was mostly driven by backwards compatibility and nice defaults. It was simple and effective to add a derive(PartialEq) to most pre v0.18 property structs and rust wouldn't allow us to add a default impl for changed() without having this bound anyway.

I think there is some discussion to be had for moving the prop replacement before the equality check and always replacing the props even in the case of "equal" props, though we would have to check for performance implications of the added mem-moves (slight, but not sure if significant) and compatibility (could the default impl of changed() to do the equality check?).