bindToBrowserHistory confuses components

[mjp0]

I'm not sure whether this is a bug or user error :)

I'm testing this mixin and I built this component:

var Test = React.createClass({
    mixins: [HistoryMixin],
    componentDidMount: function() {
        this.bindToBrowserHistory();
    },
    getInitialState: function(){
        return {elems: 0}
    },
    addE: function(){
        var new_elems = (parseInt(this.state.elems,10)+1)
        this.setState({elems: new_elems})
        this.saveState()
    },
    render: function() {
        var t = { items: {}}
        for (var i = 0; i < this.state.elems; i++) {
            t.items['item-'+i] = <div>{this.props.msg}</div>
        }
        return (
            <div className="">
                <button onClick={this.addE}>Add</button>
                {t.items}
            </div>
        )
    }
});

The problem is that if I do 2 x Test components, clicking Add on either one will add item element to both of them. I traced this back to bindToBrowserHistory method. If I comment that out, everything works, except history of course. I checked React Tools in Chrome's dev and both components have their own React root node IDs like they should.

Any idea what's causing this?

[mjp0]

Found the problem. If you inspect History.getState() you will notice that data object contains only one value elems. In practice this means that if any components share a state variable name, like elems in this case, they will all now literally share that value.

Unfortunately at current state this mixin is broken and can't be used unless you're willing to manually namespace everything with state variables.

[mjp0]

See issue #2 for a quick fix.

[leifdenby]

Thanks for your contribution @zero-, you're absolutely right that the mixin doesn't work for multiple components at the moment, which would be great to add.

I think the cleanest way to implement this would be to pass a namespacing variable to bindToBrowserHistory, which would be different for each instance of the component. I would pass in the value of this variable as a prop each time an instance of a component is created. I don't think using the rootNodeID to automatically namespace variables (as you've done in https://github.com/leifdenby/jshistory_react_mixin/pull/2) will work because as I understand it this value may change when the page is refreshed.

So my idea is to namespace by doing something like:

<MyReactComponent static_name='component1' />
<MyReactComponent static_name='component2' />

And then using static_name for namespacing in the same way as you've done in #2. Ideally the "class name" of the object would be included in the serialization too, making the full dictionary entry something like

{ myreactcomponent: { component1: { varname: 'some value' } } }

but I haven't looked into getting this from the Reactjs framework.

What do you think about this solution?

[mjp0]

I initially thought about using props as ids but abandoned the idea because it's one more thing to remember when creating a new component, and you have to do separate props with counters when looping etc. You know, general pain in the ass :)

I didn't take into account that those rootNodeIDs might change when page has been refreshed. I've been testing with such a small number of components that I suspect I might have gotten lucky in that regards. This binding issue is something that React devs would probably know better. Personally I just want to avoid as much as possible adding new stuff to add as props when creating components because essentially they become dependencies that will require maintenance.