From e6516f2d7ba3e384c72d0de53f5efaa5ffc341b6 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 8 May 2025 20:52:13 -0700 Subject: [PATCH] IntersectionObserver: Migrate js infra to shadow node family (#51148) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51148 Intersection observer should not be holding on to shadow nodes. This diff migrates the javascript infra to instead use families. Changelog: [Internal] Reviewed By: yungsters Differential Revision: D74262804 fbshipit-source-id: cc090be54f7312ce32b853ddf86567bb43e676b8 --- .../NativeIntersectionObserver.cpp | 69 ++++++++++++--- .../NativeIntersectionObserver.h | 13 +++ .../react/renderer/core/ShadowNodeFamily.h | 2 +- .../ReactNativeFeatureFlags.config.js | 10 +++ .../featureflags/ReactNativeFeatureFlags.js | 8 +- .../__tests__/IntersectionObserver-itest.js | 4 +- .../internals/IntersectionObserverManager.js | 87 +++++++++++++++---- .../specs/NativeIntersectionObserver.js | 11 +++ 8 files changed, 167 insertions(+), 37 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp index 91b441e4b5e..a24ea429097 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp @@ -21,6 +21,27 @@ NativeIntersectionObserverModuleProvider( namespace facebook::react { +namespace { + +jsi::Object tokenFromShadowNodeFamily( + jsi::Runtime& runtime, + ShadowNodeFamily::Shared shadowNodeFamily) { + jsi::Object obj(runtime); + // Need to const_cast since JSI only allows non-const pointees + obj.setNativeState( + runtime, + std::const_pointer_cast(std::move(shadowNodeFamily))); + return obj; +} + +ShadowNodeFamily::Shared shadowNodeFamilyFromToken( + jsi::Runtime& runtime, + jsi::Object token) { + return token.getNativeState(runtime); +} + +} // namespace + NativeIntersectionObserver::NativeIntersectionObserver( std::shared_ptr jsInvoker) : NativeIntersectionObserverCxxSpec(std::move(jsInvoker)) {} @@ -28,19 +49,7 @@ NativeIntersectionObserver::NativeIntersectionObserver( void NativeIntersectionObserver::observe( jsi::Runtime& runtime, NativeIntersectionObserverObserveOptions options) { - auto intersectionObserverId = options.intersectionObserverId; - auto shadowNode = - shadowNodeFromValue(runtime, std::move(options.targetShadowNode)); - auto thresholds = options.thresholds; - auto rootThresholds = options.rootThresholds; - auto& uiManager = getUIManagerFromRuntime(runtime); - - intersectionObserverManager_.observe( - intersectionObserverId, - shadowNode->getFamilyShared(), - thresholds, - rootThresholds, - uiManager); + observeV2(runtime, std::move(options)); } void NativeIntersectionObserver::unobserve( @@ -48,8 +57,40 @@ void NativeIntersectionObserver::unobserve( IntersectionObserverObserverId intersectionObserverId, jsi::Object targetShadowNode) { auto shadowNode = shadowNodeFromValue(runtime, std::move(targetShadowNode)); + auto token = + tokenFromShadowNodeFamily(runtime, shadowNode->getFamilyShared()); + unobserveV2(runtime, intersectionObserverId, std::move(token)); +} + +jsi::Object NativeIntersectionObserver::observeV2( + jsi::Runtime& runtime, + NativeIntersectionObserverObserveOptions options) { + auto intersectionObserverId = options.intersectionObserverId; + auto shadowNode = + shadowNodeFromValue(runtime, std::move(options.targetShadowNode)); + auto shadowNodeFamily = shadowNode->getFamilyShared(); + auto thresholds = options.thresholds; + auto rootThresholds = options.rootThresholds; + auto& uiManager = getUIManagerFromRuntime(runtime); + + intersectionObserverManager_.observe( + intersectionObserverId, + shadowNodeFamily, + thresholds, + rootThresholds, + uiManager); + + return tokenFromShadowNodeFamily(runtime, shadowNodeFamily); +} + +void NativeIntersectionObserver::unobserveV2( + jsi::Runtime& runtime, + IntersectionObserverObserverId intersectionObserverId, + jsi::Object targetToken) { + auto shadowNodeFamily = + shadowNodeFamilyFromToken(runtime, std::move(targetToken)); intersectionObserverManager_.unobserve( - intersectionObserverId, shadowNode->getFamilyShared()); + intersectionObserverId, shadowNodeFamily); } void NativeIntersectionObserver::connect( diff --git a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.h b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.h index d0653998093..711ebd1584e 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.h +++ b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.h @@ -61,15 +61,28 @@ class NativeIntersectionObserver public: NativeIntersectionObserver(std::shared_ptr jsInvoker); + // TODO(T223605846): Remove legacy observe method + [[deprecated("Please use observeV2")]] void observe( jsi::Runtime& runtime, NativeIntersectionObserverObserveOptions options); + // TODO(T223605846): Remove legacy unobserve method + [[deprecated("Please use unobserveV2")]] void unobserve( jsi::Runtime& runtime, IntersectionObserverObserverId intersectionObserverId, jsi::Object targetShadowNode); + jsi::Object observeV2( + jsi::Runtime& runtime, + NativeIntersectionObserverObserveOptions options); + + void unobserveV2( + jsi::Runtime& runtime, + IntersectionObserverObserverId intersectionObserverId, + jsi::Object targetToken); + void connect( jsi::Runtime& runtime, AsyncCallback<> notifyIntersectionObserversCallback); diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h index e733ac57bc6..83856089528 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h @@ -39,7 +39,7 @@ struct ShadowNodeFamilyFragment { * Represents all things that shadow nodes from the same family have in common. * To be used inside `ShadowNode` class *only*. */ -class ShadowNodeFamily final { +class ShadowNodeFamily final : public jsi::NativeState { public: using Shared = std::shared_ptr; using Weak = std::weak_ptr; diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 0cfa569445b..96b63d397ad 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -663,6 +663,16 @@ const definitions: FeatureFlagDefinitions = { }, ossReleaseStage: 'none', }, + utilizeTokensInIntersectionObserver: { + defaultValue: true, + metadata: { + dateAdded: '2025-05-06', + description: 'Use tokens in IntersectionObserver vs ShadowNode.', + expectedReleaseValue: true, + purpose: 'experimentation', + }, + ossReleaseStage: 'none', + }, }, }; diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 360a2eb2b0d..938c28f8009 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<6667fc8e4fdd2db0b54c908155b110cf>> + * @generated SignedSource<<17fa5e03fe52ed129cf731bba6e9869c>> * @flow strict */ @@ -39,6 +39,7 @@ export type ReactNativeFeatureFlagsJsOnly = $ReadOnly<{ shouldUseAnimatedObjectForTransform: Getter, shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter, shouldUseSetNativePropsInFabric: Getter, + utilizeTokensInIntersectionObserver: Getter, }>; export type ReactNativeFeatureFlagsJsOnlyOverrides = OverridesFor; @@ -155,6 +156,11 @@ export const shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter = cre */ export const shouldUseSetNativePropsInFabric: Getter = createJavaScriptFlagGetter('shouldUseSetNativePropsInFabric', true); +/** + * Use tokens in IntersectionObserver vs ShadowNode. + */ +export const utilizeTokensInIntersectionObserver: Getter = createJavaScriptFlagGetter('utilizeTokensInIntersectionObserver', true); + /** * Common flag for testing. Do NOT modify. */ diff --git a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js index c6ddb1d712e..821b34f30c6 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js @@ -7,6 +7,7 @@ * @flow strict-local * @format * @oncall react_native + * @fantom_flags utilizeTokensInIntersectionObserver:true */ import 'react-native/Libraries/Core/InitializeCore'; @@ -844,8 +845,7 @@ describe('IntersectionObserver', () => { }); }); - // TODO (T223234714): Fix memory leak and enable this test. - it.skip('should not retain initial children of observed targets', () => { + it('should not retain initial children of observed targets', () => { const root = Fantom.createRoot(); observer = new IntersectionObserver(() => {}); diff --git a/packages/react-native/src/private/webapis/intersectionobserver/internals/IntersectionObserverManager.js b/packages/react-native/src/private/webapis/intersectionobserver/internals/IntersectionObserverManager.js index c8f13649f4b..d49a08add18 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/internals/IntersectionObserverManager.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/internals/IntersectionObserverManager.js @@ -23,9 +23,11 @@ import type IntersectionObserver, { IntersectionObserverCallback, } from '../IntersectionObserver'; import type IntersectionObserverEntry from '../IntersectionObserverEntry'; +import type {NativeIntersectionObserverToken} from '../specs/NativeIntersectionObserver'; import * as Systrace from '../../../../../Libraries/Performance/Systrace'; import warnOnce from '../../../../../Libraries/Utilities/warnOnce'; +import * as ReactNativeFeatureFlags from '../../../featureflags/ReactNativeFeatureFlags'; import { getInstanceHandle, getNativeNodeReference, @@ -74,6 +76,26 @@ const targetToShadowNodeMap: WeakMap< ReturnType, > = new WeakMap(); +const targetToTokenMap: WeakMap< + ReactNativeElement, + NativeIntersectionObserverToken, +> = new WeakMap(); + +let modernNativeIntersectionObserver = + NativeIntersectionObserver == null + ? null + : NativeIntersectionObserver.observeV2 == null || + NativeIntersectionObserver.unobserveV2 == null + ? null + : { + observe: NativeIntersectionObserver.observeV2, + unobserve: NativeIntersectionObserver.unobserveV2, + }; + +if (!ReactNativeFeatureFlags.utilizeTokensInIntersectionObserver()) { + modernNativeIntersectionObserver = null; +} + /** * Registers the given intersection observer and returns a unique ID for it, * which is required to start observing targets. @@ -153,20 +175,32 @@ export function observe({ // access it even after the instance handle has been unmounted. setTargetForInstanceHandle(instanceHandle, target); - // Same for the mapping between the target and its shadow node. - targetToShadowNodeMap.set(target, targetNativeNodeReference); + if (modernNativeIntersectionObserver == null) { + // Same for the mapping between the target and its shadow node. + targetToShadowNodeMap.set(target, targetNativeNodeReference); + } if (!isConnected) { NativeIntersectionObserver.connect(notifyIntersectionObservers); isConnected = true; } - NativeIntersectionObserver.observe({ - intersectionObserverId, - targetShadowNode: targetNativeNodeReference, - thresholds: registeredObserver.observer.thresholds, - rootThresholds: registeredObserver.observer.rnRootThresholds, - }); + if (modernNativeIntersectionObserver == null) { + NativeIntersectionObserver.observe({ + intersectionObserverId, + targetShadowNode: targetNativeNodeReference, + thresholds: registeredObserver.observer.thresholds, + rootThresholds: registeredObserver.observer.rnRootThresholds, + }); + } else { + const token = modernNativeIntersectionObserver.observe({ + intersectionObserverId, + targetShadowNode: targetNativeNodeReference, + thresholds: registeredObserver.observer.thresholds, + rootThresholds: registeredObserver.observer.rnRootThresholds, + }); + targetToTokenMap.set(target, token); + } return true; } @@ -190,18 +224,33 @@ export function unobserve( return; } - const targetNativeNodeReference = targetToShadowNodeMap.get(target); - if (targetNativeNodeReference == null) { - console.error( - 'IntersectionObserverManager: could not find registration data for target', - ); - return; - } + if (modernNativeIntersectionObserver == null) { + const targetNativeNodeReference = targetToShadowNodeMap.get(target); + if (targetNativeNodeReference == null) { + console.error( + 'IntersectionObserverManager: could not find registration data for target', + ); + return; + } - NativeIntersectionObserver.unobserve( - intersectionObserverId, - targetNativeNodeReference, - ); + NativeIntersectionObserver.unobserve( + intersectionObserverId, + targetNativeNodeReference, + ); + } else { + const targetToken = targetToTokenMap.get(target); + if (targetToken == null) { + console.error( + 'IntersectionObserverManager: could not find registration data for target', + ); + return; + } + + modernNativeIntersectionObserver.unobserve( + intersectionObserverId, + targetToken, + ); + } } /** diff --git a/packages/react-native/src/private/webapis/intersectionobserver/specs/NativeIntersectionObserver.js b/packages/react-native/src/private/webapis/intersectionobserver/specs/NativeIntersectionObserver.js index e2d9a32efde..88b1a0a1529 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/specs/NativeIntersectionObserver.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/specs/NativeIntersectionObserver.js @@ -30,9 +30,20 @@ export type NativeIntersectionObserverObserveOptions = { rootThresholds?: ?$ReadOnlyArray, }; +export type NativeIntersectionObserverToken = mixed; + export interface Spec extends TurboModule { + // TODO(T223605846): Remove legacy observe method +observe: (options: NativeIntersectionObserverObserveOptions) => void; + // TODO(T223605846): Remove legacy unobserve method +unobserve: (intersectionObserverId: number, targetShadowNode: mixed) => void; + +observeV2?: ( + options: NativeIntersectionObserverObserveOptions, + ) => NativeIntersectionObserverToken; + +unobserveV2?: ( + intersectionObserverId: number, + token: NativeIntersectionObserverToken, + ) => void; +connect: (notifyIntersectionObserversCallback: () => void) => void; +disconnect: () => void; +takeRecords: () => $ReadOnlyArray;