Skip to content
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

Remove pending records when unobserving a target in IntersectionObserver #41450

Closed
wants to merge 3 commits into from

Conversation

rubennorte
Copy link
Contributor

Summary:
IntersectionObserver shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827

@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 Nov 13, 2023
@facebook-github-bot
Copy link
Contributor

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

rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 13, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 13, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
@analysis-bot
Copy link

analysis-bot commented Nov 13, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,649,388 +59
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,033,426 +26
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 686f83e
Branch: main

rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
@facebook-github-bot
Copy link
Contributor

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

rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
@facebook-github-bot
Copy link
Contributor

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

rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
@facebook-github-bot
Copy link
Contributor

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

rubennorte and others added 3 commits November 14, 2023 05:26
… Fabric (facebook#27687) (facebook#41452)

Summary:

## Summary

This fixes an error in `getPublicInstanceFromInstanceHandle` where we
throw an error when trying to access the public instance from the fiber
of an unmounted component. This shouldn't throw but return `null`
instead.

## How did you test this change?

Updated unit tests.
Before:
<img width="969" alt="Screenshot 2023-11-10 at 15 26 14"
src="https://github.com/facebook/react/assets/117921/ea161616-2775-4fab-8d74-da4bef48d09a">

After:
<img width="1148" alt="Screenshot 2023-11-10 at 15 28 37"
src="https://github.com/facebook/react/assets/117921/db18b918-b6b6-4925-9cfc-3b4b2f3ab92d">

DiffTrain build for commit facebook/react@6b3834a.

Test Plan: Sandcastle tests

Reviewed By: tyao1

Differential Revision: D51203276

Pulled By: tyao1
…k#41451)

Summary:

After [this change in React](facebook/react#27687), `ReactFabric.getPublicInstanceFromInternalInstanceHandle` can return `null` if the instance handle is a fiber that was unmounted (before that PR, it would throw an error).

This modifies the DOM traversal API to gracefully handle that case.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51210455
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
rubennorte added a commit to rubennorte/react-native that referenced this pull request Nov 14, 2023
…ver (facebook#41450)

Summary:

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Nov 14, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a493ea4.

Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
…ver (facebook#41450)

Summary:
Pull Request resolved: facebook#41450

`IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers.

Changelog: [internal]

Reviewed By: rshest

Differential Revision: D51256827

fbshipit-source-id: 28035f00bcb05a8ca53140719019032b3399436c
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 Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants