-
Notifications
You must be signed in to change notification settings - Fork 49.8k
Description
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?