clayallsopp / react.backbone

Plugin for React to make Backbone migration easier
MIT License
839 stars 60 forks source link

Performance Improvements #45

Closed ChiefORZ closed 3 years ago

ChiefORZ commented 9 years ago

Hey,

I am curious about a performance improvement in my app.

I use Backbone as my Stores and React as my View in my Flux Flow.

So i instantiate the whole Component with many different Stores.

var MainComponent = React.createClass({
    mixins: [
        React.BackboneMixin("folderStore"),
        React.BackboneMixin("itemStore")
    ],
    render: function() {
        return (
            <div className="MainComponent">
                <SubItemComponent
                  folderStore = { this.props.folderStore }
                  itemStore   = { this.props.itemStore } />
                <SecondSubItemComponent
                  folderStore = { this.props.folderStore }
                  itemStore   = { this.props.itemStore } />
            </div>
        );
    }
});

var SubItemComponent = React.createClass({
    mixins: [
        React.BackboneMixin("folderStore"),
        React.BackboneMixin("itemStore")
    ],
    render: function() {
        return (
            <div className="SubItemComponent">
                {  this.props.itemStore.get('name') }
            </div>
        );
    }
});

...

In every Component i use the Model/Collection's .get() Method to read the corresponding Field of the Store. Since I am writing/changing the Model or Collection only from within the Dispatcher... ... therefore only reading the Backbone Model or Collection from within the View...

... wouldn't it be more performant to pass the whole Collection or Model as an Object from the Top-most Parent to all it's Children? So i don't have to programmaticly call Model/Collection .get() multiple times within the View.

var MainComponent = React.createClass({
    mixins: [
        React.BackboneMixin("folderStore"),
        React.BackboneMixin("itemStore")
    ],
    render: function() {
        var folderStore = this.props.folderStore.toJSON(),
            itemStore   = this.props.itemStore.toJSON();

        return (
            <div className="MainComponent">
                <SubItemComponent
                  folderStore = { folderStore }
                  itemStore   = { itemStore } />
                <SecondSubItemComponent
                  folderStore = { folderStore }
                  itemStore   = { itemStore } />
            </div>
        );
    }
});

var SubItemComponent = React.createClass({
    render: function() {
        return (
            <div className="SubItemComponent">
                {  this.props.itemStore.name }
            </div>
        );
    }
});

...

Or is the performance difference not that big of a deal??

EtienneLem commented 9 years ago

I don’t think there’s that much of a performance issue with using Backbone’s get vs. just accessing a plain object. get is most likely pretty straightforward. But technically, I guess it is slightly faster (micro-nano-invisible-seconds wise) to access the object directly if you’re into that kind of performance improvement.

Looking at your example, MainComponent shouldn’t need the mixins array. I’m not 100% sure about what React do on a situation like that, but on a change to itemStore it renders MainComponent (which renders SubItemComponent & SecondSubItemComponent) and then each sub components also get rendered from their mixins. If you were to log every renders, I think that on a change it’d render MainComponent once and both sub components twice.

It really depends on what MainComponent job is, if all of its children listen to the same models change, then example 2 works fine. If you were to have other components inside your main that doesn’t need to render on itemStore or folderStore changes, then I’d definitely go with example 1 (without mixins).

ChiefORZ commented 9 years ago

That are very good points.

When I got a Component Chain MainComponent > SubComponent >AnotherSubComponent where SubComponent does not need to know anything about an update in itemStore, but all other Components needs to... it would trigger an Update in example 2, whereas in example 1 i can exactly define which Components needs to be updated.

Thank you for your thoughts!

ChiefORZ commented 9 years ago

and yes, you are correct. Backbone's get() Method is pretty straightforward...

get: function(attr) {
    return this.attributes[attr];
}
markijbema commented 9 years ago

I would say it''s better, but not for performance reasons. I think that way yourcomponents will work more react and less backbone that way. As a side effect it might be more performance if a change updates both a parent and it's child component (by avoiding some updates)