Skip to content

Commit

Permalink
Fix error when trying to unobserve detached targets in IntersectionOb…
Browse files Browse the repository at this point in the history
…server (#41454)

Summary:
Pull Request resolved: #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
  • Loading branch information
rubennorte authored and facebook-github-bot committed Nov 14, 2023
1 parent e8b0881 commit 11aadb7
Showing 1 changed file with 24 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof getShadowNode>, mixed],
> = new WeakMap();

/**
* Registers the given intersection observer and returns a unique ID for it,
* which is required to start observing targets.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 11aadb7

Please sign in to comment.