bitovi / ylem

Add Observable View-Models to React components
https://bitovi.github.io/ylem/
MIT License
42 stars 2 forks source link

How to handle updates within @connect #167

Closed christopherjbaker closed 6 years ago

christopherjbaker commented 6 years ago

When a Store is @connected to a Component, that observable instance gets filled with the initial props, and gets updated later when the props change. The current behaviour is that all values are overwritten when any value changes, removing any user modifications to those unchanged values.

React's suggestions for components is to be:

Of these two, being fully uncontrolled will prevent issues like "why did my data reset?". For a full reset under this option, you just need to change the key prop in the parent render.

There already exists a mapProps option which can be used to control exactly how the props are merged into the observable, allowing the developer to update some or all of the data when props change and giving them full control over the behaviour.

@mickmcgrath13 @DesignByOnyx @imaustink

BigAB commented 6 years ago

Based on article you linked to (https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html) I think being "uncontrolled by default" is a great change and very much follows the React way.

It's the point of least surprise, can be overcome by the user with a key prop, and we're still giving them a way to update based on new props in the mapProps function. Sounds like wins all around.

I support this change I support this publicly

andrejewski commented 6 years ago

Uncontrolled is also how I went for raj-react. It makes the most sense for things that are themselves stateful and aren't easily derived from the new props.

justinbmeyer commented 6 years ago

Random Justin Notes:

=======

// {outer1: "valueA", outer2: "valueB"}

setState({outer1: "VALUE"});

render(){
    <Inner inner1={this.outer1} inner2={this.outer2}>
}

var vm  = new VM({outer1: "valueA", outer2: "valueB"})
vm.inner2 = "VALUE_X";

setState({outer1: "VALUE"});

//-> {inner1: "VALUE", inner2: "valueB"}

mapProps(newProps, ){
    newProps //-> {inner1: "VALUE", inner2: "valueB"}

    {type: "add", key: "foo", value: 1},
    {type: "set", key: "foo", value: 1},
    {type: "delete", key: "foo"}
}

onClick(){
    setState({})
}

render(){
    <Inner inner1={this.outer1} inner2={this.outer2} fn={}>
}

mapProps(newProps){
    newProps //-> {inner1: "SOMETHING_NEW", inner2: "valueB"}
}

innerVM.inner1 = "foo";
fn(){
    this.props.outer1 = "SOMETHING_NEW";
}

I think shallow diff is ok for a default