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

Do child components need @observer? #101

Closed
tiagogm opened this issue Jan 29, 2016 · 26 comments
Closed

Do child components need @observer? #101

tiagogm opened this issue Jan 29, 2016 · 26 comments

Comments

@tiagogm
Copy link

tiagogm commented Jan 29, 2016

Hello,

I've been experimenting with mobserbables for a few days, and ran into an issue, but I'm not quite sure it's a bug or by design

(I'm new to react as well)

I currently have two components, a container and a child.
The container is passed a store that contains a observable array, and then is passes that array into the child.

A bit like this:
//main.jsx

<div>
<container store={store}/>
</div>

//container.jsx

//in the constructor or container.jsx I mutate the array
setTimeout( () =>  this.props.store.markers.newMarker() )
//or which mutates marker[0] for example
setTimeout( () =>  this.props.store.markers.moveMarker() )
let {zoom, markers} = this.props.store;
<div>
<p> zoom info from store {this.props.zoom}</p>
<child markers={markers}/>
</div>

//child.jsx

let {markers} = this.props
<div>
{
   markers.map( (m) => {
      return (<p> lat={m.lat} lng={m.lng}  </p>)
   })
}
</div>

I'm trying to re-render the child when I change that array on the container, so if I push a new element, or a change a current one the child.jsx will re-render to reflect that change.

The problem is the child.jsx does not re-render
However if I add the same loop in the container.jsx render(), it does.

//container.jsx

let {zoom, markers} = this.props.store;
<div>
<p> zoom info from store {this.props.zoom}</p>
{
   markers.map( (m) => {
      return (<p> lat={m.lat} lng={m.lng}  </p>)
   })
}
<child markers={markers}/>
</div>

This way the child.jsx will re-render.
Is this by design? I cannot observe an array form a child unless the parent is directly dependent (uses it on it's own render()) on it?

Thank you.

@mweststrate
Copy link
Member

Could you include the full code listings?

@tiagogm
Copy link
Author

tiagogm commented Jan 29, 2016

I was putting a complete example up on fiddle, when I realised I did not include the @observer decorator in the ``child.jsx` component.

Now it appears to work fine.
Sorry wasting your time!

@tiagogm tiagogm closed this as completed Jan 29, 2016
@hc-12
Copy link

hc-12 commented Feb 28, 2016

i'm a little bit confused, is @observer decorator needs be decorated in the children component as well?

say if i have a container with @observer, and pass down observable array to its children. Those children would need an @observer decorator as well?

@mweststrate
Copy link
Member

yes, @observer gives components the ability to track their own observables and render independently of their parent component, which has huge performance benefits.

@hc-12
Copy link

hc-12 commented Feb 28, 2016

So what's the best practice for mobx? Would you recommend to decorate @observer where ever a component touches an observable?

@mweststrate
Copy link
Member

exactly.

@hc-12
Copy link

hc-12 commented Feb 28, 2016

Well that clears things up.

Thanks

@luisherranz
Copy link
Member

Michel, is there any performance drawback if you use @observer in a component which doesn't get any observable?

Imagine we make all our app components observer by default. Is there any benefit on manually choosing those which are actually using observables?

@mweststrate
Copy link
Member

The performance of adding @observer is neglectable and should in practice not be measurable, so just passing them all through observer should work fine (we do that as well in some projects). Bear in mind though that @observer automatically applies the shouldComponentUpdate from PureRenderMixin. (Yes, mobx-react applies the same optimizations as immutable data based implementations! Everyone that sees why that is possible has truly grokked mobx(-react) :)).

If it would be convenient to you or anyone else feel free to file a feature request for a base ObserverReactComponent class that can be inherited from, as alternative to applying @observer.

@mweststrate mweststrate changed the title Observable arrays Do child components need @observer? Feb 28, 2016
@mweststrate mweststrate reopened this Feb 28, 2016
@tiagogm
Copy link
Author

tiagogm commented Feb 29, 2016

That seems like a nice idea.

I actually used something like that, since we don't need state at all we can actually use stateless components syntax.

//MobxObserver.jsx
const MobxObserver = mobxReact.observer((props) => <div>{props.children}</div>);
export default MobxObserver;

Then in my component

import MobxComponent from 'components/MobxObserver';
class MapContainer extends MobxObserver {
///..etc
}

@luisherranz
Copy link
Member

Interesting, thanks!

@vladimir-rovensky
Copy link

Yes, mobx-react applies the same optimizations as immutable data based implementations! Everyone that sees why that is possible has truly grokked mobx(-react) :)

Could you elaborate on that? How is it possible to shallow compare the observable props if they are not immutable?

@mweststrate
Copy link
Member

if the object is mutable but observable, children don't need to be updated as they are already observing the object.

Say you have a Task object t1 which is passed by TodoListView to TodoView as prop. TodoView will start observing the relevant properties of t1. So when t1 changes, TodoListView doesn't need to know nor pass it again to the TodoView. Because if the change is relevant, TodoView will re-render anyways. TodoView only needs to be signaled by the parent if it tries to pass another todo to it. But that will correctly be picked up, as t1 and t2 are not the same references.

@vladimir-rovensky
Copy link

Ok, one thing I'm still not getting is implementation of shouldComponentUpdate of TodoView in this example. If t1 changes some of its observable properties, it is still the same instance of Task. I assume since t1 is observable, TodoView is notified of change and re-renders. Does that mean that TodoView's shouldComponentUpdate is bypassed in this case? Normally it would determine that t1 is the same instance as before and skip the rendering...

@mweststrate
Copy link
Member

@vladimir-rovensky exactly. It knows that it has to rerender, so the shouldComponentUpdate is skipped.

@vladimir-rovensky
Copy link

Very nice, thank you for explaining.

@mweststrate
Copy link
Member

Btw, it is explained in more detail here: https://www.mendix.com/tech-blog/making-react-reactive-pursuit-high-performing-easily-maintainable-react-apps/. It explains also how it results in 100% non-wasted renderings.

@dacz
Copy link

dacz commented May 21, 2016

I know that's closed but I'm still a little bit confused.

When I use observe on parent, parent gets the updated data from observable store and may pass them to the children. Even when children do not have observe wrapping, they should update (new props should be passed down). At least when I modify your fiddle https://jsfiddle.net/mweststrate/wv3yopo0/ and remove observe from TodoView, it still updates itself and updates the count, too.

I understand that for performance reasons it is good to decorate children, because they may update without updating the parent, right? So when I removed the observe decorator, I forced the parent to rerender on change (it rerenders anyway because the count, but if the count would not be there, it would not be re-rendered if todo is decorated...?)

For example when I have the component to render a list of items that are supposed not to change (like listing of products) and my parent component (still sticking to HoC) just decide what list (different filters, sorting etc.) to display, the list doesn't have to be decorated (but as I see it has no measurable negative effect on performace it it is). Is it right?

When I would compute (as observable aggregated data) the list in this HoC the list according to (router paths and query), and pass this computed list to the children (to display this list), they still do not need to be decorated? Or they has to be because they observe the computed value from HoC?

I cannot believe that MobX exists because I see it as a dreams come true. I like Redux but sometimes I'm lost in all dispatching, reducing, thunks and normalized store. MobX seems to be so intuitive and straightforward. Thanks a lot for your work.

@mweststrate
Copy link
Member

Lot of questions, I'll try to answer them with a generic explanation. You can best see @observer of a variation of @computed (it calculates a VDOM based on some observable data structures)

Take a look at the following example:

class Person {
  @observable age = 30
  @observable firstName = "Foo"
  @observable lastName = "Bar"

  @computed /*?*/ get displayName() {
    console.log("calculating displayName")
    return this.firstName + this.lastName
  }
}

var p = new Person();

autorun(() => {
  console.dir(p.age + p.displayName)
})

p.firstName =  "John"
p.age = 30;

Now should displayName have @computed or not? The runtime behavior with and without will be exactly the same. Except for the logging statements! Without the @computed, the displayName expression return this.firstname + this.lastName will be tracked as part of the autorun, autorun accesses displayName, which accesses first / lastName, so MobX knows that the autorun depends on those two observables.

However, if @computed is present, MobX tries to track displayName as separate expression. So the autorun no longer depends on age/firstName/lastName, but on age/displayName.

That means that if the age is modified, the autorun will re-run, grabbing the latest valeus from age and displayName. However the displayName expression won't be re-run because it knows its own dependencies (firstName / lastName) didn't change, so the cached result can be returned. In contrast to no @computed, where for each autorun the expression return this.firstName + this.lastName needs to be re-evaluated, even if just the age changed, because get displayName is just a normal JS function call.

You can best see this whole mechanism as a function stack (it actually is); when an observable is read it will always be associated with the function on top of the stack. The more often you push another function on top of the stack, the less work each function has to do (less data to track). All variations of autorun, computed and observer add a new entry to the stack, and thereby reduce the workload of their parent and adding an opportunity for MobX to cache intermediate results. Results that reduce the need to recompute everything and can be reused by others (for example another computed / autorun that also uses displayName) It's really a divide and conquer approach.

The same holds for @observer. It is not always needed technically, but skipping it makes the render function of the component 'part' of the render function of the parent component. This means that the parent might need to re-render more often and track more data.

The only use case for not using @observer is if there are no observables used in the render at all; all objects are passed in plain etc. This might indeed be the case if the parent component does all the calculations (like filtering) and pass on the filtered lists. But if the filtered, normal js array still contains observable data structures, I suggest to still add @observer so that the parent doesn't need to track those objects.

A bit elaborate and I hope that helps to answer your question by giving some insight in what happens behind the scenes. Otherwise just let me know :)

@dacz
Copy link

dacz commented May 23, 2016

@mweststrate That's excellent answer, thank you! I'm getting into it and I like the approach a lot.

@dcadenas
Copy link

dcadenas commented Oct 3, 2016

@mweststrate I think you meant to change the age in the last line of your example to show how the autorun uses the cached latest value of displayName as then neither firstName nor lastName are changed, something like: p.age = 31;

@stgolem
Copy link

stgolem commented Jul 21, 2017

Can anyone please provide me an example of using mobx with big 3-rd party component library like ant-design?
We are stuck with Table and all its nested components. Here is basic example:

@observer
export class ProcessList extends React.Component<ProcessListProps, ProcessListState> {
    constructor(props: ProcessListProps) {
        super(props);
        this.state = {
            ProcessList: (this.props[processlist] as ProcessListService).getProcessTree,
            isLoading: (this.props[processlist] as ProcessListService).isLoading,
            modelVisible: false
        };
    }

    componentWillReceiveProps(nextProps: ProcessListProps) {
        this.setState({
            ProcessList: (this.props[processlist] as ProcessListService).getProcessTree,
            isLoading: (this.props[processlist] as ProcessListService).isLoading
        });
        return true;
    }
    
    editProcess = (record) => {
        this.setState({modelDialog: record, modelVisible: true});
    }

    render() {
        const { ProcessList } = this.state;
        
        return (
            <div style={{ textAlign: 'center' }}>
                <Table rowKey={(record) => record.Uid} dataSource={ProcessList}>
                    <Column key="Name" title="Name" dataIndex="Name" />
                    <Column key="edit" title="edit" render={(text, record) => (
                    <span>
                    <Button onClick={() => this.editProcess(record)}>Edit</Button>
                    </span>
                )} /> 
                </Table>
                <ModalComponent model={this.state.modelDialog} visible={this.state.modelVisible}/>
            </div>
        );
    }
}

As you can see here we have Table and Column that is bound to dataIndex by property name. To make it work we have to recompile whole ant-design project, that have references on more internal https://github.com/react-component components (we have to mark them as observer also).

How one can wrap ALL external components without recompilation?
Isn't it confusing and misleading with whole MobX concept?
Can we remove this limitation from our mobx sources?

@asfernandes
Copy link

asfernandes commented Nov 13, 2017

If the component (using @observer) has properties that the parent pass inline arrow functions, does it means the child will always re-render when the parent re-renders?

@asfernandes
Copy link

Answering myself, yes, an arrow function created inside the parent render method will make the child to re-render every time the parent renders. To avoid that we can create a method or arrow function as class member.

smiclea pushed a commit to smiclea/coriolis-web that referenced this issue Mar 29, 2018
Adding `@observer` to all React components can have a great positive
impact on performance. You can see more info about it in this discussion
with an owner of mobx: mobxjs/mobx#101
smiclea pushed a commit to smiclea/coriolis-web that referenced this issue Mar 29, 2018
Adding `@observer` to all React components can have a great positive
impact on performance. You can see more info about it in this
discussion: mobxjs/mobx#101
sfentress added a commit to concord-consortium/geocode that referenced this issue Apr 18, 2019
Since these components are passed an observable array, they won't
re-render automatically when the array's internals update, and instead
only re-render when a primitive value updates. This means that their
rendering is always a step behind in the simulation.

This makes the less pure-componenty, but seems to be the recommended way.
See:

mobxjs/mobx-state-tree#1060
https://github.com/mobxjs/mobx-react#faq
mobxjs/mobx#101

An alternative would be to use `getSnapshot(data)`, but this is claimed
to be less efficient. (Untested, and perhaps for our use-case is negligible
and the cleaner code would be preferred.)
@exapsy
Copy link

exapsy commented Feb 2, 2020

Pardon me, I am just a little bit confused but I get what should I do to fix this issue.
I'm confused when it comes to components. A component in a generic "pages/components" type of architecture is supposed to not know about the application's state.

const Main = observer(() => {
    const todoStore = useContext(TodoStore)

    return (
        <div className='page main'>
            {/* Note: This component doesn't get updated - unless parent component re-renders */}
            <Todo
                todoList={todoStore.todos}
                onNewTodo={(val) => todoStore.addTodo(val)}
                onModifyTodo={(index, val) => todoStore.modifyTodo(index, val)}
                onDeleteTodo={(index) => todoStore.removeTodo(index)}
            />
        </div>
    )
})
This works btw
const Main = observer(() => {
    const todoStore = useContext(TodoStore)

    return (
        <div className='page main'>
            <ul
                className='todo__list'
            >
                {todoStore.todos.map((todo, index) => (
                    <li
                        className='todo__list__item'
                        key={index}
                    >
                        {todo.task}
                    </li>
                ))}
            </ul>
        </div>
    )
})

So, in that type of architecture what are you supposed to do? Or if you're using a UI framework like Semantic where the framework has no knowledge of what state manager you're using?

@danielkcz
Copy link
Contributor

Please, refrain from commenting on old and closed issues as only a few people will see that. Open new issue with all necessary details.

@mobxjs mobxjs locked as resolved and limited conversation to collaborators Feb 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests