Skip to content

Fabric: Export shadow node family to javascript #51146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

RSNara
Copy link
Contributor

@RSNara RSNara commented May 6, 2025

Summary:
InteresectionObserver caches shadow nodes in javascript.

This is hazardous: caching shadow nodes could lead to memory leaks.

Therefore, this diff exports shadow node families to javascript via fabric ui manager.

This way, we don't have to cache the shadow nodes in js.

Changelog: [

Reviewed By: yungsters

Differential Revision: D74262805

RSNara and others added 8 commits May 6, 2025 13:04
Differential Revision: D73874822
Differential Revision: D74095745
Differential Revision: D74095747
Summary: The dev bundle has leaks.

Differential Revision: D74095744
Summary:
Currently, `IntersectionObserver#observe` retains a reference to the `ShadowNode` of the view that is supplied as an argument, which is used to compute intersection whenever a shadow tree is committed.

However, the `shadowNode` includes all of child nodes and state at the time that `IntersectionObserver#observe` is called. This means that an active `IntersectionObserverEntry` will retain references to memory that would otherwise be deallocated (e.g. if children of the observed view are unmounted after `IntersectionObserver#observe` is called).

This diff refactors `IntersectionObserver` to instead retain a reference to the `ShadowNodeFamily`, which does not retain references to child nodes and still eanbles `IntersectionObserver` to compute intersections.

Changelog:
[General][Changed] - Fixed `IntersectionObserver#observe` to avoid retaining memory for unmounted child nodes of observed views.

Differential Revision: D74130479
Summary:
IntersectionObserverManager now uses families to manage IntersectionObservers.

This diff just migrates its apis to family too.

Changelog: [Internal]

Differential Revision: D74263375

Reviewed By: yungsters
Summary:
InteresectionObserver caches shadow nodes in javascript.

This is hazardous: caching shadow nodes could lead to memory leaks.

Therefore, this diff exports shadow node families to javascript via fabric ui manager.

This way, we don't have to cache the shadow nodes in js.

Changelog: [

Reviewed By: yungsters

Differential Revision: D74262805
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner labels May 6, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D74262805

RSNara added a commit to RSNara/react-native that referenced this pull request May 6, 2025
…1146)

Summary:

InteresectionObserver caches shadow nodes in javascript.

This is hazardous: caching shadow nodes could lead to memory leaks.

Therefore, this diff exports shadow node families to javascript via fabric ui manager.

This way, we don't have to cache the shadow nodes in js.

Changelog: [

Reviewed By: yungsters

Differential Revision: D74262805
@javache
Copy link
Member

javache commented May 9, 2025

Closed internally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants