salsita / spicy-hooks

7 stars 0 forks source link

Usefulness of `core/useValueVersion` #27

Closed goce-cz closed 3 years ago

goce-cz commented 3 years ago

I'm starting to question the usefulness of useValueVersion hook outside of its only usage within the library - the useInternedValue

Somehow I'm leaning towards the opinion that useInternedValue would simply do better job anywhere the useValueVersion comes in mind.

Example

const MyComponent = ({deepObject}) => {
  const deepObjectVersion = useValueVersion(deepObject, isEqual)
  useEffect(
    () => console.log('the object changed deeply', deepObject),
    [deepObjectVersion] // linter warns about wrong dependency list
  )
  ...
}

can easily be replaced with probably more obvious and way more React friendly

const MyComponent = ({deepObject}) => {
  const internedDeepObject = useInternedValue(deepObject, isEqual)
  useEffect(
    () => console.log('the object changed deeply', internedDeepObject),
    [internedDeepObject ] // all fine here
  )
  ...
}

Both hooks are very simple, so I'd be keen to merge them if we decide not to export the useValueVersion.

What do you think? Should we remove 👍 or not 👎 ?

Fezzzi commented 3 years ago

Based on the described use case of useValueVersion, it indeed seems that useInternedValue could fully replace it when needed. I can't think of any reasonable use case for actual number of times the value was changed that couldn't be handled by higher-level useInternedValue. The fact that it removes linter warning might be something to consider as well. Let's do it 💪

goce-cz commented 3 years ago

Related issue: #28