Component data being reused when it shouldn't.

[bryceosterhaus]

This is similar to #123

class GrandChild extends Component {
    render() {
        return (
            <div class="GrandChild">GrandChild</div>
        );
    }
}

class Child1 extends Component {
    render() {
        return (
            <GrandChild elementClasses="one" />
        );
    }
}

class Child2 extends Component {
    render() {
        return (
            <GrandChild elementClasses="two" />
        );
    }
}

class Parent extends Component {
    handleClick() {
        this.state.bool = !this.state.bool;
    }

    render() {
        let RetComponent = Child1;

        if (this.state.bool) {
            RetComponent = Child2;
        }

        return (
            <div onClick={this.handleClick.bind(this)}>
                <RetComponent />
            </div>
        );
    }
}

Parent.STATE = {
    bool: {
        value: false
    }
};

On first render, the classes on the GrandChild element will be GrandChild one. Then when you click on the container, the new classes will be GrandChild one two. I would expect the classes to just be GrandChild two. I believe the GrandChild components data is being re-used when it shouldn't.

Let me know if you need clarifying. Thanks!

[mairatma]

Thanks for reporting this, the example you gave was very helpful in reproducing the issue.

I've investigated this and confirmed that this is not actually caused by reusing data, the data is actually correct for the second call. This is being caused because of the way this default elementClasses prop is implemented, without incremental dom, which can lead to problems like this. I believe this is the same thing causing an issue that @eduardolundgren mentioned to me recently too.

I'll make sure to fix this so that it won't conflict with incremental dom's logic anymore.

[eduardolundgren]

Should we remove elementClasses?

[mairatma]

That's what I was thinking right now, not sure if it's very useful, though I do see people using it every now and then.

Regardless, that would be a breaking change... It'd be safer to only remove it on the next version.

[bryceosterhaus]

We use elementClasses quite a bit. It is very useful in adding classes to the outer most component element.

[mairatma]

Awesome, we should keep it then, just need to make sure it works well with incremental dom (when we first added it we weren't using incremental dom yet).

[bryceosterhaus]

elementClasses is also a nice utility that react doesn't provide. A lot of our react components had to manually set a className prop and it was very annoying because we had to manually merge classes with it. So this was one thing I really liked about Metal when we started using it.

If the naming convention needs to change, I see no issue with that though.

[mairatma]

Don't worry, the necessary changes are all internal, you shouldn't notice it (besides the issue being fixed, of course 😄)

[mairatma]

This has been fixed in version 2.2.0.