From 024f3357bb69d3f49c26449a81375ca5f0436e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 7 May 2024 09:43:48 -0700 Subject: [PATCH] Fix bug in IntersectionObserver (#44439) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/44439 Changelog: [internal] (IntersectionObserver isn't enabled yet in OSS). This fixes a bug in `IntersectionObserver` when observing the same target in multiple observers. In that case, the first time we `unobserve` we clean up some metadata that's shared across observers, and other observers observing the target have problems with the missing data. This fixes the problem by removing the clean up, as the data structure backing this information is a `WeakMap` anyway, so it'll be cleaned up automatically eventually, and the stored data is very small. Reviewed By: twobassdrum Differential Revision: D57046864 fbshipit-source-id: b001cf1ae4f4c91b74b1ad487e01691d5f3be1ce --- .../IntersectionObserverManager.js | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js b/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js index 4d50e668e1a..d1ebf5d70aa 100644 --- a/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js +++ b/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js @@ -67,19 +67,11 @@ function setTargetForInstanceHandle( instanceHandleToTargetMap.set(key, target); } -function unsetTargetForInstanceHandle(instanceHandle: mixed): void { - // $FlowExpectedError[incompatible-type] instanceHandle is typed as mixed but we know it's an object and we need it to be to use it as a key in a WeakMap. - const key: interface {} = instanceHandle; - instanceHandleToTargetMap.delete(key); -} - // The mapping between ReactNativeElement and their corresponding shadow node // also needs to be kept here because React removes the link when unmounting. -// We also keep the instance handle so we don't have to retrieve it again -// from the target to unobserve. -const targetToShadowNodeAndInstanceHandleMap: WeakMap< +const targetToShadowNodeMap: WeakMap< ReactNativeElement, - [ReturnType, mixed], + ReturnType, > = new WeakMap(); /** @@ -163,12 +155,8 @@ 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 - // and instance handle. - targetToShadowNodeAndInstanceHandleMap.set(target, [ - targetShadowNode, - instanceHandle, - ]); + // Same for the mapping between the target and its shadow node. + targetToShadowNodeMap.set(target, targetShadowNode); if (!isConnected) { NativeIntersectionObserver.connect(notifyIntersectionObservers); @@ -201,26 +189,18 @@ export function unobserve( return; } - const targetShadowNodeAndInstanceHandle = - targetToShadowNodeAndInstanceHandleMap.get(target); - if (targetShadowNodeAndInstanceHandle == null) { + const targetShadowNode = targetToShadowNodeMap.get(target); + if (targetShadowNode == null) { console.error( 'IntersectionObserverManager: could not find registration data for target', ); return; } - const [targetShadowNode, instanceHandle] = targetShadowNodeAndInstanceHandle; - NativeIntersectionObserver.unobserve( intersectionObserverId, targetShadowNode, ); - - // We can guarantee we won't receive any more entries for this target, - // so we don't need to keep the mappings anymore. - unsetTargetForInstanceHandle(instanceHandle); - targetToShadowNodeAndInstanceHandleMap.delete(target); } /**