Memoize connect response callback functions

[sdemjanenko]

I ran into this issue when using the file-upload package, but I think it applies to all components. The code examples

const api = fileUpload.connect(state, send, normalizeProps)

returns a new api object every time a component renders. Ideally this would be memoized so child components can optimize their rendering if they so choose. Memoizing the callback functions separately would also be useful. I ran into an issue where I had code that updated the machine's state if a parent component provided a new value

useEffect(() => {
  api.setFiles(value)
}, [value, api.setFiles])

Because api.setFiles is new every time the component renders, the useEffect gets run (even when the value is unchanged).

I think adding memoization into the connect methods would be really helpful. This probably means the connect methods need to become hooks in React.

[segunadebayo]

Hi @sdemjanenko.

For this use case, I'd recommend using useMemo to memorize the file upload's connect function.

const api = useMemo(() => fileUpload.connect(state, send, normalizeProps), [state])

Another approach is to hold the api in a ref to avoid adding it to dependencies.

const apiRef = useRef()
const api = fileUpload.connect(state, send, normalizeProps)
apiRef.current = api

useEffect(() => {
  apiRef.current.setFiles(value)
}, [value])

You could combine both approaches for slightly better results.

[segunadebayo]

We don't want to memoize functions in the api because different frameworks handle "reactivity" differently, and we don't want to get into the business of synchronizing our memo implementation with the frameworks'.

[sdemjanenko]

The problem with this is that every time state changes, the callback functions will get regenerated because they are not memoized deeply.

[segunadebayo]

I don’t see how this is a problem except if you have performance reproduction for me to take a look at.

The main fabric of React is based on the model of re-rendering. Re-renders are not an enemy and not bad in that sense. It only becomes an issue if it leads to an obvious bottleneck.

https://thoughtbot.com/blog/react-rendering-misconception

We don’t plan to deeply memoize the functions because every framework has a custom way they handle reactivity and state tracking. This is a rabbit hole and might lead to staleness within the callbacks if not done correctly.

Hope this helps.

[sdemjanenko]

Its not so much of a performance issue as that it breaks useEffect and other situations where the memoization key is changing unexpectedly. As a consumer of high-quality 3rd party packages, I generally expect those packages to consider whether their response structure is ideal for intended uses. I would argue that the connect return type will cause issues for many React users, especially when they are trying to sync state with a state manager outside of the Zag machine.