fix: avoid NotFoundError when the web measurement element is already detached - #746
Open
wildan-m wants to merge 1 commit into
Open
Conversation
…ched The web provider removes its hidden measurement element with document.body.removeChild(element), which throws NotFoundError when the element is no longer a child of body. Anything that mutates document.body — an extension, a page-translation layer, a third-party script — can detach it before the effect cleanup runs, and the throw escapes React's passive-unmount chain as an uncaught error. Detach the listener first, then use element.remove(), which is a no-op when the node has no parent. The attached case is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On web,
NativeSafeAreaProviderappends a hidden measurement element todocument.bodywhen it mounts, and its effect cleanup removes it again:Node.removeChild()throwsNotFoundErrorwhen the node is not currently a child of the node it is called on. So if anything detaches that element before the cleanup runs — a browser extension, the browser's page-translation layer, or any third-party script that mutatesdocument.body— the cleanup throws. Because it runs inside React's passive-unmount chain, nothing catches it and it surfaces as an uncaught error:This changes the cleanup to detach the listener first and then use
element.remove(), which is a no-op when the node has no parent:Two small notes on the shape of the fix:
element.remove()is preferred over guarding withdocument.body.contains(element)because it also handles the case where the element was re-parented rather than removed.Behaviour is unchanged when the element is still attached — it is removed exactly as before. Only the previously-throwing path differs.
We hit this in production at Expensify, reported via Sentry: Expensify/App#95343
Test Plan
yarn testpasses (prettier, eslint, tsc, jest).The failing path is a teardown race, so it has to be set up explicitly. Mount the provider, detach the measurement element the way an external script would, then unmount so React runs the cleanup:
unmount()throwsNotFoundError: The node to be removed is not a child of this node.unmount()completes with no error.Running the same steps with the element left attached succeeds both before and after, which confirms the normal path is untouched.