Closed lschuft closed 4 years ago
Hi @lschuft, thanks for the detailed report!
This is expected behaviour since onDrawingLoad
and onPrepareSVG
are both recreated each time DesignViewer
renders, meaning they're different objects and therefore not referentially equal.
It's the same reason why this happens:
(() => {}) === (() => {})
// Output: false
It's something to be aware of when using React, but most of the time it's not an issue as you've found out.
One way around this is to use the useCallback
hook:
const onPrepareSVG = React.useCallback((svg: SVGElement) => {
svg.classList.add(classes.design);
}, [classes.design]);
const onDrawingLoad = React.useCallback((error: Error | null) => {
if (error === null) {
if (svgDrawingManager.current === null && svgComponent.current) {
svgDrawingManager.current = new SvgDrawingManager(
svgComponent.current.container as HTMLDivElement
);
}
dispatch(AppActions.enumerateRegions(svgDrawingManager.current!.getDrawing()));
}
}, [
/*
I'm not exactly sure what should go into this dependency array, so you'll probably
have to play around with that. If you've got the ESLint `exhaustive-deps` rule set
up, that should help guide you.
*/
svgDrawingManager.current,
svgComponent.current,
AppActions.enumerateRegions
]);
This creates memoized versions of those callbacks that won't change across re-renders of DesignViewer
, meaning ReactSVG
will see these as unchanged.
Kent Dodds has a nice article on this stuff if you want to dig deeper here.
Another option would be to reorganise the component tree somehow. This might involve wrapping ReactSVG
in another component, and having that component accept the extra props the callbacks are interested in. It's a bit more work though which is why I suggested hooks first. Happy to expand on this if you want me to though?
Anyway, keen to hear how you get on 🙏
Thanks for the detailed response, @tanem ! The provided article was very useful. I preferred to use the useCallback approach; I also learned that useCallback is also necessary when using the loading and fallback props.
Thanks again for the small React lesson, closing this as I managed to get it working. Cheers!
No probs, glad to hear it 🎉
Hi, thanks for this library.
I'm having an issue when a component, with a ReactSVG in its tree, re-renders. The re-render triggers because of a state change unrelated to the props passed to ReactSVG, but it nevertheless causes ReactSVG to trigger again the injection.
I'm using both afterInjection to execute some logic and beforeInjection to implement a bit of styling. When the parent component re-renders, it understandably triggers the componentDidUpdate of ReactSVG. But the shallowDiffers function considers that both callbacks functions changed, so it triggers removeSVG() and renderSVG() which in turn call the before and afterInjection callbacks causing a bit of havoc in my app.
Here's the parent component:
When regions change (Line 7; I'm using it in a piece of code not included here), Redux triggers a re-render. But as you can see, I'm never changing the beforeInjection or afterInjection code; still the code in shallowDiffers consider that they changed:
Is this a problem with that function, or I'm somehow passing the callbacks incorrectly? If, when debugging, I do a manual check between prevProp and this.props (prevProps.afterInjection == this.props.afterInjection) it still fails, but this is how I pass callbacks to all components and I never had this issue.