kettanaito / atomic-layout

Build declarative, responsive layouts in React using CSS Grid.
https://redd.gitbook.io/atomic-layout
MIT License
1.13k stars 33 forks source link

useResponsiveValue might require to fire an effect for a dependency change #326

Closed hallaji closed 4 years ago

hallaji commented 4 years ago

When:

The useResponsiveValue hook does not update the value if one of the dependencies changes. Internationalisation is a good example for this situation.

const Name: FC<Props> = ({ firstname, fullname }) => {
  return <span>{useResponsiveValue({ xs: firstname }, fullname)}</span>
}

Expected behavior:

An effect should be fired to update the value conditionally.

  useEffect(() => {
    setValue(withBreakpoints<ValueType>(breakpoints, defaultValue))
  }, [defaultValue, breakpoints])

Environment:

kettanaito commented 4 years ago

The fix is published in 0.16.2. Could you please update and let me know if that worked? Thanks.

hallaji commented 4 years ago

@kettanaito Apparently, this works only if we narrow down the value type to primitive types. Otherwise, the setValue in useEffect results in an infinite loop (e.g. a value type such as a react node or element).

Imagine you switch to a new locale, a new default value and breakpoints object will be passed in. So I think we need a sort of side effect to update the breakpoints for useBreakpointChange.

kettanaito commented 4 years ago

@hallaji hey, thanks for following with this. Can this issue be solves by using something like useDeepCompareEffect?

hallaji commented 4 years ago

not quite sure. I'll take a look.