From 11aadb72876c923ad099a504ad7b28afdbc4ef5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 14 Nov 2023 06:11:13 -0800 Subject: [PATCH] Fix error when trying to unobserve detached targets in IntersectionObserver (#41454) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41454 Yet another problem caused by React being too aggressive cleaning up detached fibers. This fixes a problem in `IntersectionObserver` when trying to `unobserve` detached targets. In that case we didn't have access to its shadow node anymore, so we didn't have a way to unobserve in native. This keeps an additional mapping in JS to do the conversion even after detached. Changelog: [internal] Reviewed By: rshest Differential Revision: D51257960 fbshipit-source-id: 25edc9afd2108e141d178dd4939fc2de8286342b --- .../IntersectionObserverManager.js | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js b/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js index 3401553ad0e..892fce941c1 100644 --- a/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js +++ b/packages/react-native/Libraries/IntersectionObserver/IntersectionObserverManager.js @@ -70,6 +70,15 @@ function unsetTargetForInstanceHandle(instanceHandle: mixed): void { 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< + ReactNativeElement, + [ReturnType, mixed], +> = new WeakMap(); + /** * Registers the given intersection observer and returns a unique ID for it, * which is required to start observing targets. @@ -151,6 +160,13 @@ 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, + ]); + if (!isConnected) { NativeIntersectionObserver.connect(notifyIntersectionObservers); isConnected = true; @@ -182,30 +198,26 @@ export function unobserve( return; } - const targetShadowNode = getShadowNode(target); - if (targetShadowNode == null) { + const targetShadowNodeAndInstanceHandle = + targetToShadowNodeAndInstanceHandleMap.get(target); + if (targetShadowNodeAndInstanceHandle == null) { console.error( - 'IntersectionObserverManager: could not find reference to host node from target', + 'IntersectionObserverManager: could not find registration data for target', ); return; } + const [targetShadowNode, instanceHandle] = targetShadowNodeAndInstanceHandle; + NativeIntersectionObserver.unobserve( intersectionObserverId, targetShadowNode, ); - const instanceHandle = getInstanceHandle(target); - if (instanceHandle == null) { - console.error( - 'IntersectionObserverManager: could not find reference to instance handle from target', - ); - return; - } - // We can guarantee we won't receive any more entries for this target, - // so we don't need to keep the mapping anymore. + // so we don't need to keep the mappings anymore. unsetTargetForInstanceHandle(instanceHandle); + targetToShadowNodeAndInstanceHandleMap.delete(target); } /**