search

vasturiano / react-kapsule

React wrapper for kapsule-style web components
MIT License
8 stars 2 forks source link

Improper `useEffect` #14

Closed cjkoepke closed 3 weeks ago

cjkoepke commented 3 weeks ago

This line is causing Maximum depth exceeded errors downstream: https://github.com/vasturiano/react-kapsule/blob/master/src/index.js#L35

Should be:

useEffect(() => setPrevProps(props), [props]); // remember previous props
vasturiano commented 3 weeks ago

@cjkoepke thanks for reaching out.

The useState should skip the re-render if the values are equal, as mentioned in the docs: https://react.dev/reference/react/useState

If the new value you provide is identical to the current state, as determined by an Object.is comparison, React will skip re-rendering the component and its children.

Therefore, having the dependency on the useEffect or not should in principle cause no difference in the behaviour. Unless the input props keep changing, but in that case having the dependency would also not help, that would be a downstream issue.

Would you be able to make an example online that reproduces the issue? Like on https://codesandbox.io/ for instance.

Thanks!

cjkoepke commented 3 weeks ago

Ah, this is a good point. The error for me is coming from another library that is using react-kapsule, so there's a chance the props are not being handled correctly. Thanks for the prompt and detailed reply!