From 5fc2bd4f6071db9a8fed28e698025d4ceffe2bf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 22 Aug 2024 12:23:37 -0700 Subject: [PATCH] Improve handling of disconnected nodes in MutationObserver (#46157) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46157 Changelog: [internal] This improves the handling of disconnected nodes in `MutationObserver`. Specifically: * When observing a node, if the node is disconnected (unmounted) this is just a no-op (without logging errors). We can't observe an unmounted node. * When disconnecting the observer, if the observed nodes are disconnected, we get the target shadow node from an internal map, which we always have access to if we successfully started observing the node. If this logs an error now, it's something to look into but it won't generally log it if the target is just disconnected. That will work correctly. Reviewed By: bvanderhoof Differential Revision: D61655856 fbshipit-source-id: d18a885350ef000fc563c85f6775ba864d184ad1 --- .../mutationobserver/MutationObserver.js | 6 ++-- .../MutationObserverManager.js | 29 ++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/react-native/src/private/webapis/mutationobserver/MutationObserver.js b/packages/react-native/src/private/webapis/mutationobserver/MutationObserver.js index 1ee166b78cd..cb170f30837 100644 --- a/packages/react-native/src/private/webapis/mutationobserver/MutationObserver.js +++ b/packages/react-native/src/private/webapis/mutationobserver/MutationObserver.js @@ -121,13 +121,15 @@ export default class MutationObserver { MutationObserverManager.unobserve(mutationObserverId, target); } - MutationObserverManager.observe({ + const didStartObserving = MutationObserverManager.observe({ mutationObserverId, target, subtree: Boolean(options?.subtree), }); - this._observationTargets.add(target); + if (didStartObserving) { + this._observationTargets.add(target); + } } _unobserve(target: ReactNativeElement): void { diff --git a/packages/react-native/src/private/webapis/mutationobserver/MutationObserverManager.js b/packages/react-native/src/private/webapis/mutationobserver/MutationObserverManager.js index 3d2c5d9eb7e..68367c30c28 100644 --- a/packages/react-native/src/private/webapis/mutationobserver/MutationObserverManager.js +++ b/packages/react-native/src/private/webapis/mutationobserver/MutationObserverManager.js @@ -43,6 +43,13 @@ const registeredMutationObservers: Map< $ReadOnly<{observer: MutationObserver, callback: MutationObserverCallback}>, > = new Map(); +// The mapping between ReactNativeElement and their corresponding shadow node +// needs to be kept here because React removes the link when unmounting. +const targetToShadowNodeMap: WeakMap< + ReactNativeElement, + ReturnType, +> = new WeakMap(); + /** * Registers the given mutation observer and returns a unique ID for it, * which is required to start observing targets. @@ -85,10 +92,10 @@ export function observe({ mutationObserverId: MutationObserverId, target: ReactNativeElement, subtree: boolean, -}): void { +}): boolean { if (NativeMutationObserver == null) { warnNoNativeMutationObserver(); - return; + return false; } const registeredObserver = @@ -97,17 +104,17 @@ export function observe({ console.error( `MutationObserverManager: could not start observing target because MutationObserver with ID ${mutationObserverId} was not registered.`, ); - return; + return false; } const targetShadowNode = getShadowNode(target); if (targetShadowNode == null) { - console.error( - 'MutationObserverManager: could not find reference to host node from target', - ); - return; + // The target is disconnected. We can't observe it anymore. + return false; } + targetToShadowNodeMap.set(target, targetShadowNode); + if (!isConnected) { NativeMutationObserver.connect( notifyMutationObservers, @@ -121,11 +128,13 @@ export function observe({ isConnected = true; } - return NativeMutationObserver.observe({ + NativeMutationObserver.observe({ mutationObserverId, targetShadowNode, subtree, }); + + return true; } export function unobserve( @@ -146,10 +155,10 @@ export function unobserve( return; } - const targetShadowNode = getShadowNode(target); + const targetShadowNode = targetToShadowNodeMap.get(target); if (targetShadowNode == null) { console.error( - 'MutationObserverManager: could not find reference to host node from target', + 'MutationObserverManager: could not find registration data for target', ); return; }