Hi, we encountered this warning
Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of withBoundingRects() which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://fb.me/react-strict-mode-find-node
in div (created by Tooltip)
in Tooltip (created by TooltipWithBounds)
in TooltipWithBounds (created by withBoundingRects())
Is it possible to replace findDOMNode with a ref in WrappedComponent.componentDidMount here?
Previous discussion: https://github.com/hshoff/vx/pull/509#discussion_r331679227
Hey @sosloow, I would love to help. Can you please explain the scope of the changes like have to update this accross the repo or just few files?
@hshoff I have made some changes to replace the findDOMNode with ref. I tried to test with jest but doesn't get any idea. Can you please help me how I can test that changes doesn't breaking things? thanks :)
Came across this same issue when using <TooltipWithBounds />:
https://codesandbox.io/s/discontinuous-dates-visx-3ve9k?file=/src/App.js
If I had a suggestion about how to overcome this it would be to pass a ref from the bounding element:
const ref = useRef()
const {} = useTooltip({containerRef: ref})
return (
<div>
<svg ref={ref}>
...
</svg>
<TooltipWithBounds />
</div>
)
Using a ref is definitely the right fix here (and this is now what we do for useTooltipInPortal), but we'd like to do it in a way that isn't a breaking change just yet. This probably means that we need a way to support both (in both @visx/bounds and in @visx/tooltip where it's used).
One option would be to leave the withBoundingRects as is, and introduce a new hook useBoundingRects + a new NewTooltipWithBounds (with better name) which could require a ref/ResizeObserver polyfill (see below) and not be a breaking change. In a future breaking change these would then become the defaults:
@visx/bounds changes
// @visx/bounds
import useMeaure from 'react-use-measure';
// `useMeasureOptions` may include `ResizeObserver` polyfill else it must be
// defined globally
function useBoundingRects(useMeasureOptions) {
const [privateContainerRef, containerBounds] = useMeasure(useMeasureOptions);
const [privateParentRef, parentBounds] = useMeasure(useMeasureOptions);
// set both container + parent container in public ref
const containerRef = useCallback(
(container: HTMLElement | SVGElement | null) => {
privateContainerRef(container);
privateParentRef(container?.parentElement ?? null);
},
[privateContainerRef, privateParentRef],
);
return { containerRef, containerBounds, parentBounds };
}
@visx/tooltip changes
// @visx/tooltip
import { useBoundingRects } from `@visx/bounds';
// a new component avoids a breaking change because `useBoundingRects` / `react-use-measure`
// requires ResizeObserver polyfill to be passed or defined globally
function NewTooltipWithBounds(useMeasureOptions) {
const { containerRef, containerBounds, parentBounds } = useBoundingRects(useMeasureOptions);
// ...bounds detection logic in existing `TooltipWithBounds`
return (
<Tooltip ref={containerRef} {...}>{...}</Tooltip>
);
}
and finally usage
// your app
function MyChart() {
// ...
return (
<div>
<svg>{...}</svg>
<NewTooltipWithBounds />
</div>
);
}
Most helpful comment
Using a
refis definitely the right fix here (and this is now what we do foruseTooltipInPortal), but we'd like to do it in a way that isn't a breaking change just yet. This probably means that we need a way to support both (in both@visx/boundsand in@visx/tooltipwhere it's used).One option would be to leave the
withBoundingRectsas is, and introduce a new hookuseBoundingRects+ a newNewTooltipWithBounds(with better name) which could require aref/ResizeObserverpolyfill (see below) and not be a breaking change. In a future breaking change these would then become the defaults:@visx/boundschanges@visx/tooltipchangesand finally usage