observer: props is, itself, observable when only each prop should be

[adamabeshouse]

EDITED because original query was not coherent:

In this jsfiddle, there's a toy model of a scenario I'm facing: I have multiple subcomponents in render which I want to not re-compute unnecessarily, and I was hoping to be able to use the observable nature of props together with the Observer tag to implement this. However, because its not the properties of props which are tracked, but the reference to this.props itself, both subcomponents will recompute every time any props change, regardless of which props are actually used in each subcomponent.

The class in the fiddle is duplicated below:

@observer 
class Person extends Component<{ firstName:string, lastName:string }, {}> {
    constructor(props) {
        super(props);
        this.getFirstName = ()=>{
            return (<span>{this.props.firstName}</span>);
        };
        this.getLastName = ()=>{
            return (<span>{this.props.lastName}</span>);
        };
    }
    render() {
        return (
          <div>
              <Observer>{this.getFirstName}</Observer>
              <Observer>{this.getLastName}</Observer>
          </div>
        );
    }
}

There are two possible solutions to regain the desired functionality:
(1) Use components (decorated with @observer), and pass in desired data through props, in order to regain the shallow comparison functionality in shouldComponentUpdate, e.g.

@observer
class FirstName extends Component<{name:string},{}>{

...

<FirstName name={this.props.firstName}/>

(2) Wrap all prop accesses in @computed's, e.g.

 this.getFirstName = ()=>{
            return (<span>{this.firstName}</span>);
};


@computed get firstName() {
    return this.props.firstName;
}

Both are more unwieldy than if the props object were observable, thus each prop tracked individually, instead of the reference to the props object.

[mweststrate]

Could you setup a fiddle / sandbox demonstrating this issue? Op do 19 okt. 2017 om 00:35 schreef Adam Abeshouse <notifications@github.com

…

: @observer class Person extends React.Component<{ firstName:string, lastName:string }, {}> { render() { return <span>{this.props.firstName}</span>; } } This will rerender when props.lastName changes, even though there's no need for it to, because this.props object has an atom associated to it that reports change https://github.com/mobxjs/mobx-react/blob/master/src/observer.js#L143 This makes using props quite unwieldy - in order to avoid this issue I'd need to wrap every prop access in a @computed to prevent rerender unless the value changes. For example, this works as desired/expected: @observer class Person2 extends React.Component<{ firstName:string, lastName:string }, {}> { @computed get firstName() { return this.props.firstName; } render() { return <span>{this.firstName}</span>; } } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#347>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhHtvElyA2kDDxQbmShwO_9x3ohXBks5stn1IgaJpZM4P-dFk> .

[adamabeshouse]

@mweststrate sorry for the sloppy initial query, please see my updated post above.

[urugator]

The problem is that props object is immutable and the whole props object is replaced when new props arrives. The props are not updated individually, therefore making them observable is useless.
The relevant discussion can be found here

[adamabeshouse]

@urugator that's a good point. However, even so, it could be accomplished using getters and setters.

[adamabeshouse]

It could also be accomplished by having an observable props object which is, key by key, updated w the values of the immutable object coming in.

[urugator]

Prop keys are dynamic, keys of observable object not...

[adamabeshouse]

This could be handled with observable.map

[urugator]

How? You need to translate this.props.something into this._propsMap.get("something")

[adamabeshouse]

EDIT: @urugator pointed out this wouldn't work, please see updated proposal.

---

Actually, observable.map wouldn't be necessary. The code could look something like this (note that observer.js already does something like this for "props", just not per-key):

const valueHolder = {};
const setProxy = {};
const getProxy = {};

Object.defineProperty(this, "props", {
    configurable: true,
    enumerable: true,
    get: function() {
        return getProxy; // separated from setProxy to make props read-only by users
    },
    set: function set(v) {
        for (const key of Object.keys(v)) {
            if (!getProxy.hasOwnProperty(key)) {
                installKey(getProxy, setProxy, valueHolder, key);
            }
            setProxy[key] = v[key];
        }
    }
});

function installKey(getProxy, setProxy, valueHolder, key) {
    const atom = new Atom("reactive props."+key);
    Object.defineProperty(setProxy, key, {
        configurable: true,
        enumerable: false,
        set: function set(v) {
            if (valueHolder[key] !== v) {
                valueHolder[key] = v;
                atom.reportChanged();
            }
        }
    });
    Object.defineProperty(getProxy, key, {
        configurable: true,
        enumerable: true,
        get: function() {
            atom.reportObserved();
            return valueHolder[key];
        },
    });
}

[urugator]

You need to "subscribe" also for prop keys, which were used in render, but weren't passed by props yet. These keys should be observed, but are not installed yet, therefore the subscription won't happen.
Or am I missing something?

[adamabeshouse]

Good catch. I wasn't aware of this functionality but it looks like you can also define getters with computed property names. So here's an updated (simpler) version that handles the issue you raised - an atom is created the first time the key is accessed or set, and reportChanged()'ed whenever the value changes. Otherwise, it works the same as it currently does - the props object is immutable and the new values completely replace the old ones:

const atoms = {};
let valueHolder = {};

function getAtom(atoms, key) {
    atoms[key] = atoms[key] || new Atom("reactive props."+key);
    return atoms[key];
}

function union(strList1, strList2) {
    // returns unique elements of strList1+strList2
}

Object.defineProperty(this, "props", {
    configurable: true,
    enumerable: true,
    get [key]() {
        getAtom(atoms, key).reportObserved();
        return valueHolder[key];
    },
    set: function set(v) {
        for (const key of union(Object.keys(v), Object.keys(valueHolder))) {
            if (valueHolder[key] !== v[key]) {
                getAtom(atoms, key).reportChanged();
            }
        }
        valueHolder = v;
    }
});

[urugator]

getters with computed property names

Computed prop name is not a param passed to function. It simply means that you can do this:

let propName = a ? "a" : "b";
const o = { 
  get [propName]() {....} // propName must be defined here
}

The Proxies are the only way to do this.

[adamabeshouse]

@urugator ah my mistake. Yes Proxy seems to be what I'm looking for. I think it's clear that would be possible with not much change to the code I've written above.

In my opinion this is the way observable props should work - I can't think of a case where someone would want to observe changes to props without observing changes to a particular prop, and by observing changes to particular props we get the fine-grained behavior we need for the application I brought up in my OP. The only potential pitfalls as I see are that (1) This is more computational work on prop access and on setting props, but that may be negligible, and (2) That this would be a breaking change. (2) could be solved by adding a decorator argument to determine "observable props mode", to maintain backwards compatibility.

[urugator]

Unfortunately, we can't use Proxy currently.

Note that what you do is not a typical or recommeded usage.
You should not pass primitive values (string/reference/etc), but observable objects and dereference them as late as possible (when the values are actually needed).
That way you will not only prevent unnecessary re-render of subcomponent, but also the parent component (or any intermediate component), which re-renders unnecessarily as well.
The @observer is usually completely useless if your component does not accept any observable object (like in your example) - making props observable is not the point of @observer.
The standard (React) way to achieve the desired behavior is to make your subcomponents extends React.PureComponent (or implement shouldComponentUpdate if required).

Which doesn't mean that it can't be useful in some situations...

[adamabeshouse]

That makes sense, thanks for your responses.