maslianok / react-resize-detector

A Cross-Browser, Event-based, Element Resize Detection for React
http://maslianok.github.io/react-resize-detector/
MIT License
1.25k stars 91 forks source link

bad setState #170

Closed Karpengold closed 3 years ago

Karpengold commented 3 years ago

Hello, I'm curious if it is possible to change state inside onResize event, cause now I'm getting the warning: index.js:1 Warning: Cannot update a component while rendering a different component

I'm trying to create a component with 2 different states, if users add new child and there is not enough space for children it looks like firstChild, ... , endChild and it displays all children otherwise. So I created state variable and check if there is enough space using ref:

 const onResize = useCallback(() => {
    if (targetRef.current) {
      const { offsetWidth, scrollWidth } = targetRef.current;
      if (offsetWidth < scrollWidth) {
        setOverflowed(true);
      } else {
        setOverflowed(false);
      }
    }
  }, [targetRef]);
SamStonehouse commented 3 years ago

I think maybe you could just do

const { width, height } = useResizeDetector();
const { offsetWidth, scrollWidth } = targetRef.current;
const overflowed = offsetWidth < scrollWidth;

And then used overflowed directly without state?

Alternatively, if you need to use the callback, maybe you could use variables defined with useRef(), so you wouldn't have to use setState?

I have a the same issue but using state though a React Context so I think I have to use setState, did you figure out any other solution?

stuartkeith commented 3 years ago

I'm not sure if this is the best way, but I worked around this by using an intermediate local state that is then dispatched via useLayoutEffect.

function Component({ setParentState }) {
  const [height, setHeight] = useState(undefined);

  useResizeDetector({
    // setParentState can't be called here as parent is still rendering
    onResize: () => setHeight(el.offsetHeight) 
  });

  useLayoutEffect(() => {
    setParentState(height);
  }, [height]);
}
maslianok commented 3 years ago

Sorry for the late answer, I hope the problem is already resolved.

In general, I agree with @SamStonehouse - you should try to avoid using extra hooks and creatting additional state variables.

In the example above you can call setParentState(el.offsetHeight) inside useResizeDetector, right? And you can remove height, setHeight variables

maslianok commented 3 years ago

Let me know if you have any additional questions. I'm going to close the issue

stuartkeith commented 3 years ago

In the example above you can call setParentState(el.offsetHeight) inside useResizeDetector, right? And you can remove height, setHeight variables

No, you can't, that's why the workaround is necessary in the first place. This doesn't work:

function Component({ setParentState }) {
  useResizeDetector({
    onResize: () => setParentState(el.offsetHeight) 
  });
}

The reason is onResize is triggered immediately, before the parent component has finished rendering. If you try to change a parent's state while the parent is still rendering, React throws the Warning: Cannot update a component while rendering a different component exception.

This limitation doesn't apply to state in the same component though. By adding the local state, you let the parent component finish rendering before you then dispatch the value via useLayoutEffect.

maslianok commented 3 years ago

I see... Yeah, working with DOM elements in React sometimes become tricky :)