web-ridge/react-ridge-state

Selector function and variables inside are memoized

Closed this issue · 10 comments

Consider the following example: https://codesandbox.io/s/jolly-archimedes-hooy6

It looks like the very first selector callback is memoized and outside value changes are not considered.

I think this is a similar problem as useEffect, useCallback are having, that's why they introduced the dependency array. I think react-ridge-state would also need a dependency array to make it work.

Like so

const content = pages.useSelector((state) => {
  return state[page]
}, [page])

Either that, or it would be necessary to not memoize the selector func and put the responsibility into userland and suggest to use useCallback:

const content = pages.useSelector(useCallback((state) => state[page], [page]))

Oh wow, interesting. I like the redux api so maybe we should remove the useCallback internally since the end result will be memoized anyways because of the useState and the equals check

Should be fixed in 4.1.7

Thanks for your quick response @RichardLindhout! The change makes sense. Unfortunately, it still doesn't seem to work in the CodeSandbox: https://codesandbox.io/s/jolly-archimedes-hooy6

I've updated the version and put a console.log inside the selector function, it seems to be triggered, but the resulting value pageContent does not.

It does not work indeed

Thank you for providing feedback. It works now!
https://codesandbox.io/s/proud-platform-28s6d?file=/package.json

Thanks again for your response and quick release, it works now! :)

Sorry to hijack this issue again 😅…

But now another issue came up: We are frequently selecting from array collections, but the equality function seems to be too simple to distinguish these values.
As a result, we now would need to add an equality function for every selector there is. Can we have a way to set a default one?

Illustrating the problem here: https://codesandbox.io/s/practical-napier-0grus
You can uncomment line 26 to make it work. Either way, I don't think it should result in an endless re-render loop

Oh wow thanks for finding the endless re-render bug.

Fixed the endless re-render loop in 4.2.0. For the rest you should consider using React.memo for the children or the deepEqual check (like in the example below)

Since most of the time the re-renders of many children will be heavy and the container element won't matter that much.

Updated example:
https://codesandbox.io/s/zen-swartz-j38e3?file=/package.json

Thank you so much for your quick action (and sorry for the headaches I might be causing 😅). Still enjoying your library a lot 🙌!

No problem thanks for your feedback I really like it!