Visx: ReactDOM.findDOMNode() in @vx/bounds

Created on 2 Jun 2020  路  5Comments  路  Source: airbnb/visx

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?

breaking changes needs followup 馃憢help wanted

Most helpful comment

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>
  );
}

All 5 comments

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>
  );
}
Was this page helpful?
0 / 5 - 0 ratings

Related issues

hshoff picture hshoff  路  12Comments

rd-dev-ukraine picture rd-dev-ukraine  路  37Comments

AJK92 picture AJK92  路  5Comments

alangrn picture alangrn  路  10Comments

hshoff picture hshoff  路  8Comments