From c120ccb7eed1fca227108849ef807d508cab5487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 29 Nov 2023 12:00:29 -0800 Subject: [PATCH] Small refactor of image attached callbacks logic (#41701) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41701 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 fbshipit-source-id: 472836840b19958402bd0de3e2c09c7cec004156 --- .../Libraries/Image/Image.android.js | 13 +----- .../react-native/Libraries/Image/Image.ios.js | 12 +---- .../Libraries/Image/ImageInjection.js | 46 ++++++++++++------- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/packages/react-native/Libraries/Image/Image.android.js b/packages/react-native/Libraries/Image/Image.android.js index eeec72e2724aea..e5c5d9f223b616 100644 --- 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 | null>( - // $FlowFixMe[incompatible-call] - forwardedRef, - // $FlowFixMe[incompatible-call] - imageAttachedCallbacksRef, - ); + const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef); return ( diff --git a/packages/react-native/Libraries/Image/Image.ios.js b/packages/react-native/Libraries/Image/Image.ios.js index 2dabd798485eb5..ce21b5b4edf049 100644 --- 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 | null>( - // $FlowFixMe[incompatible-call] - forwardedRef, - // $FlowFixMe[incompatible-call] - imageAttachedCallbacksRef, - ); + const actualRef = useWrapRefWithImageAttachedCallbacks(forwardedRef); return ( diff --git a/packages/react-native/Libraries/Image/ImageInjection.js b/packages/react-native/Libraries/Image/ImageInjection.js index 7a2e59ca9a45a3..780743abbe9a5e 100644 --- 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 { +export function useWrapRefWithImageAttachedCallbacks( + forwardedRef: React.RefSetter, +): React.RefSetter { const pendingCleanupCallbacks = useRef 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 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( + // $FlowFixMe[incompatible-call] + forwardedRef, + // $FlowFixMe[incompatible-call] + imageAttachedCallbacksRef.current, + ); }