fix(Image): keep function identiy accross renders

[Gregoirevda]

Description

Currently when an inline function is passed to the Image or Animated.Image component, the useEffect will be triggered on every render, causing the image the be fetched on every render.

props accepting a function and declared in the dependency array are: onLoad onLoadEnd onLoadStart onLoadError.
It shouldn't be up to the developer to pass a memoized function.

The browser will cache subsequent image fetches (not sure about all platforms), so as it may seem acceptable setting the cache key to reload will fetch the image on every render.

Example

function App() {
  const Component = Image || Animated.Image;
 
  // Not memoized, causing the fetch on every render
  function bad() {}
  
  const good = useCallback(() => {}, [])

  return <Component source={{uri}} onLoadEnd={bad} onLoad={() => { /* equally bad */ }} onLoadStart={good)  />
}

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9e05b48:

Sandbox	Source
react-native-web-examples	Configuration

[Gregoirevda]

@necolas Does the fix make sense? I didn't know where to put the utility method so added in vendor to get feedback, but tell me where the right place is please

[lkinasiewicz]

Is there any point delaying the ref update through an effect?

[Gregoirevda]

initial ref is set with const callbackRef = useRef(callback);

it then updates in the effect

[lkinasiewicz]

I understand that. I mean, you can simply update the ref without any effects. Simpler code and it guarantees that the most recent value of callback is assigned to the ref and will be called by Image - you don't know when exactly the effect will be executed and Image may stop loading between the render and the effect.

[Gregoirevda]

That's true. I'm wondering why swr does it this way though? (They also don't have the config in the dependency array). There's also the experimental_useEffectEvent from react, so I guess there are edge cases with both solutions