Small refactor of image attached callbacks logic (#41701)

Summary: I did a hotfix for this logic in D51618512. This does a small refactor to improve the code (moving more shared code to the hook and avoiding creating a closure unnecessarily in every call to it). Changelog: [internal] Reviewed By: javache Differential Revision: D51660288
facebook · Nov 29, 2023 · 16f27f2 · 16f27f2
1 parent 5476121
commit 16f27f2
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 37 deletions.
diff --git a/packages/react-native/Libraries/Image/Image.android.js b/packages/react-native/Libraries/Image/Image.android.js
@@ -15,11 +15,10 @@ import type {AbstractImageAndroid, ImageAndroid} from './ImageTypes.flow';
 import flattenStyle from '../StyleSheet/flattenStyle';
 import StyleSheet from '../StyleSheet/StyleSheet';
 import TextAncestor from '../Text/TextAncestor';
-import useMergeRefs from '../Utilities/useMergeRefs';
 import ImageAnalyticsTagContext from './ImageAnalyticsTagContext';
 import {
   unstable_getImageComponentDecorator,
-  useRefWithImageAttachedCallbacks,
+  useWrapRefWithImageAttachedCallbacks,
 } from './ImageInjection';
 import {getImageSourcesFromImageProps} from './ImageSourceUtils';
 import {convertObjectFitToResizeMode} from './ImageUtils';
@@ -200,15 +199,7 @@ let BaseImage: AbstractImageAndroid = React.forwardRef(
     const resizeMode =
       objectFit || props.resizeMode || style?.resizeMode || 'cover';
 
-    const imageAttachedCallbacksRef = useRefWithImageAttachedCallbacks();
-
-    const actualRef =
-      useMergeRefs<React.ElementRef<AbstractImageAndroid> | null>(
-        // $FlowFixMe[incompatible-call]
-        forwardedRef,
-        // $FlowFixMe[incompatible-call]
-        imageAttachedCallbacksRef,
-      );
+    const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef);
 
     return (
       <ImageAnalyticsTagContext.Consumer>

diff --git a/packages/react-native/Libraries/Image/Image.ios.js b/packages/react-native/Libraries/Image/Image.ios.js
@@ -15,11 +15,10 @@ import type {AbstractImageIOS, ImageIOS} from './ImageTypes.flow';
 import {createRootTag} from '../ReactNative/RootTag';
 import flattenStyle from '../StyleSheet/flattenStyle';
 import StyleSheet from '../StyleSheet/StyleSheet';
-import useMergeRefs from '../Utilities/useMergeRefs';
 import ImageAnalyticsTagContext from './ImageAnalyticsTagContext';
 import {
   unstable_getImageComponentDecorator,
-  useRefWithImageAttachedCallbacks,
+  useWrapRefWithImageAttachedCallbacks,
 } from './ImageInjection';
 import {getImageSourcesFromImageProps} from './ImageSourceUtils';
 import {convertObjectFitToResizeMode} from './ImageUtils';
@@ -162,14 +161,7 @@ let BaseImage: AbstractImageIOS = React.forwardRef((props, forwardedRef) => {
   };
   const accessibilityLabel = props['aria-label'] ?? props.accessibilityLabel;
 
-  const imageAttachedCallbacksRef = useRefWithImageAttachedCallbacks();
-
-  const actualRef = useMergeRefs<React.ElementRef<AbstractImageIOS> | null>(
-    // $FlowFixMe[incompatible-call]
-    forwardedRef,
-    // $FlowFixMe[incompatible-call]
-    imageAttachedCallbacksRef,
-  );
+  const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef);
 
   return (
     <ImageAnalyticsTagContext.Consumer>

diff --git a/packages/react-native/Libraries/Image/ImageInjection.js b/packages/react-native/Libraries/Image/ImageInjection.js
@@ -14,6 +14,7 @@ import type {
   Image as ImageComponent,
 } from './ImageTypes.flow';
 
+import useMergeRefs from '../Utilities/useMergeRefs';
 import * as React from 'react';
 import {useRef} from 'react';
 
@@ -52,24 +53,37 @@ export function unstable_unregisterImageAttachedCallback(
   imageAttachedCallbacks.delete(callback);
 }
 
-export function useRefWithImageAttachedCallbacks(): React.RefSetter<ImageInstance> {
+export function useWrapRefWithImageAttachedCallbacks(
+  forwardedRef: React.RefSetter<ImageInstance>,
+): React.RefSetter<ImageInstance> {
   const pendingCleanupCallbacks = useRef<Array<() => void>>([]);
 
-  const ref = useRef((node: ImageInstance | null) => {
-    if (node == null) {
-      if (pendingCleanupCallbacks.current.length > 0) {
-        pendingCleanupCallbacks.current.forEach(cb => cb());
-        pendingCleanupCallbacks.current = [];
-      }
-    } else {
-      imageAttachedCallbacks.forEach(imageAttachedCallback => {
-        const maybeCleanupCallback = imageAttachedCallback(node);
-        if (maybeCleanupCallback != null) {
-          pendingCleanupCallbacks.current.push(maybeCleanupCallback);
+  const imageAttachedCallbacksRef =
+    useRef<?(node: ImageInstance | null) => void>(null);
+
+  if (imageAttachedCallbacksRef.current == null) {
+    imageAttachedCallbacksRef.current = (node: ImageInstance | null): void => {
+      if (node == null) {
+        if (pendingCleanupCallbacks.current.length > 0) {
+          pendingCleanupCallbacks.current.forEach(cb => cb());
+          pendingCleanupCallbacks.current = [];
         }
-      });
-    }
-  });
+      } else {
+        imageAttachedCallbacks.forEach(imageAttachedCallback => {
+          const maybeCleanupCallback = imageAttachedCallback(node);
+          if (maybeCleanupCallback != null) {
+            pendingCleanupCallbacks.current.push(maybeCleanupCallback);
+          }
+        });
+      }
+    };
+  }
 
-  return ref.current;
+  // `useMergeRefs` returns a stable ref if its arguments don't change.
+  return useMergeRefs<ImageInstance | null>(
+    // $FlowFixMe[incompatible-call]
+    forwardedRef,
+    // $FlowFixMe[incompatible-call]
+    imageAttachedCallbacksRef.current,
+  );
 }