facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.88k stars 47.06k forks source link

React 16.3.1, setState of parent onMouseEnter in child incredibly sluggish #12782

Closed CSMR-DB closed 6 years ago

CSMR-DB commented 6 years ago

Issue Type: Bug?

What is the current behavior?

I'm refactoring code from my graph Components, built with d3. I am using Composition to create an instance that looks somewhat like this:

<Vizzy>
    <VizzyHeader title="Guests staying" />
    <VizzySVG>
        <Line
            data={data}
        />
        <Bars
            data={data}
            isHelper
        />
    </VizzySVG>
    <VizzyLegend />
</Vizzy>

I have defined a method on "Vizzy" which lets me set the current index of the data shown in the graph and I am passing that as a function to the child Components. Like so:

class Vizzy extends Component {
    state = {
        currentActiveData: "Value"
    };
    setActiveData = dataIndex => {
        this.setState({
            currentActiveData: dataIndex
        });
    };
    render() {
        const { currentActiveData } = this.state;
        const setActiveData = this.setActiveData;

        const childrenWithProps = React.Children.map(
            this.props.children,
            child =>
                React.cloneElement(child, { currentActiveData, setActiveData })
        );
        return <VizzyContainer>{childrenWithProps}</VizzyContainer>;
    }
}

It then gets passed to the child "VizzySVG", because that's where I need the method to run onMouseEnter. So, a little bit of prop drilling to make it look like this:

The data from Vizzy's state gets passed to "VizzyLegend", because that's where I ultimately need the data to be available.

class VizzySVG extends Component {
    state = { vizzyWidth: 0, vizzyHeight: 0 };
    componentDidMount = () => {
        this.setState({
            vizzyWidth: this.refs.VizzyRef.offsetWidth,
            vizzyHeight: this.refs.VizzyRef.offsetHeight
        });
    };
    render() {
        const { vizzyWidth, vizzyHeight } = this.state;
        const { setActiveData, currentActiveData } = this.props;

        const childrenWithProps = React.Children.map(
            this.props.children,
            child =>
                React.cloneElement(child, {
                    vizzyWidth,
                    vizzyHeight,
                    setActiveData,
                    currentActiveData
                })
        );

        return (
            <div ref="VizzyRef" style={{ display: "flex", height: "100%" }}>
                <Svg
                    width={vizzyWidth}
                    height={vizzyHeight}
                    viewBox={`0 0 ${vizzyWidth} ${vizzyHeight}`}
                >
                    {childrenWithProps}
                </Svg>
            </div>
        );
    }
}

Then, in Bars.jsx, I call the method onMouseEnter. The major issue is that it is extremely sluggish / slow. Even causing issues with the CSS to transition on :hover. If I put the that method as an onClick, the problem is gone. However, that eliminates the intended behavior I'm going for. When constructing a similar graph using my old Component built with d3 to handle interaction (updating the DOM), this problem doesn't arise. But that code is less scalable and maintainable, so I need to get this refactor to work.

Also, when I just use onMouseEnter={() => console.log("hovered")} inside Bars.jsx, the console just updates as expected and animation is fluent. No issues with CSS :hover whatsoever.

I would expect the data to get updated fluently when using the onMouseEnter to setState(), just like updating the DOM using d3 or even jQuery would. I figured drilling props just 2 levels would not be an issue, albeit not ideal.

This is running React 16.3.1 on Ubuntu 16.04. I don't know about the behavior in older versions of React because I only started to refactor after updating to 16.3.x.

EDIT: This may just be an issue with my version of Firefox for some reason, the behavior in Chrome is much smoother. Will check if updating Firefox helps.

EDIT 2: It seems that updating Firefox has helped and that it can no longer be considered a bug. It is now performing pretty much on par with Chrome, yet it is still slower than using d3 to directly manipulate the DOM using d3's .html(). Is this as much as I can expect from using prop drilling and setting state from nested components using just React?

gaearon commented 6 years ago

Sorry, it’s hard to guess without actually running the code.

gaearon commented 6 years ago

Oops, didn’t mean to close. But we’ll have to close if you can’t provide an example we could run and try.

CSMR-DB commented 6 years ago

I will attempt to provide an example tomorrow. Unfortunately I only got time on Wednesdays and Thursdays to work on my project at the moment.

CSMR-DB commented 6 years ago

Update: I seem to have pretty much resolved the issue by using shouldComponentUpdate() in "VizzySVG". Performance is now on a level I was expecting. Epiphany strikes yet again. My next step would now be to try to implement either Context or Redux to lift state management from the root Component, possibly using some sort of namespacing if needed. This would at least be the case for Redux, as far as I've been able to find. Closing this issue.