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

refactor[DebuggingRegistry]: highlight nodes only on the lowest container for legacy implementations #41843

Closed
wants to merge 11 commits into from

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Dec 7, 2023

Summary:
Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added isChildPublicInstance from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

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

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

@analysis-bot
Copy link

analysis-bot commented Dec 7, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,582,044 +4,090
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 19,955,024 -18
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: af8c56a
Branch: main

Ruslan Lesiutin and others added 11 commits January 2, 2024 04:15
Summary:
Changelog: [Internal]

There will be a single DebuggingRegistry instance per runtime, which will be responsible for finding lowest AppContainer ancestor for highligthed component.

It will receive refs to inspected views (ancestors) as subscriptions and later will call all necessary methods.

For some methods actual implementation will be published in the next diffs.

Differential Revision: https://internalfb.com/D51536787

fbshipit-source-id: 904db278d20a2570c41680434b8c3c527582521e
Summary: Changelog: [Internal]

Differential Revision: https://internalfb.com/D51603860

fbshipit-source-id: 6e5bbcf9ab04f85a7be1717afdb46b14c6dd3578
…ents

Summary: Changelog: [Internal]

Differential Revision: https://internalfb.com/D51603861

fbshipit-source-id: 9bdffd6dc97e9458d8273cd57a9a3a274058a63f
…vTools

Summary: Changelog: [Internal]

Differential Revision: https://internalfb.com/D51708053

fbshipit-source-id: a40890db1de96a977bf302ed21db4e0b1f9443e3
Summary: Changelog: [Internal]

Differential Revision: https://internalfb.com/D51708055

fbshipit-source-id: dd5075f63de5cc08c93579719d72f18d0a909894
Summary: Changelog: [Internal]

Differential Revision: https://internalfb.com/D51708054

fbshipit-source-id: 38ed14fa87b013f2670a9387e3694ff68014af90
Summary:
Changelog: [Internal]

TODO: add a link to github issue about brownfield app being broken in dev mode

Differential Revision: https://internalfb.com/D50644900

fbshipit-source-id: 3a529dad516f686d965e022fc48714d2af4a8aa7
Summary:
Changelog: [Internal]

Forking implementations for trace updates and element highlights from React DevTools: modern and legacy.

Both implementations will later solve the same problem of highlighting the component only on a single AppContainer, but with different approaches:
- Modern will be based on DOM Node APIs: `getBoundingClientRect` and `parentElement`.
- Legacy will be based on `isChildInstance` from renderer and `measure`.

All corresponding API call will be added in a separate diff later on top of these changes.

Differential Revision: https://internalfb.com/D51713087

fbshipit-source-id: ba2e3a4f15e63480dd982f2f87a955c2367aebb4
…iner for modern implementations

Summary:
Changelog: [Internal]

Use `parentElement` API to find lowest AppContainer ancestor, which will be responsible for highlighting an inspected element or rendering trace updates frames on the screen.

Differential Revision: https://internalfb.com/D51713089

fbshipit-source-id: ad7f532e5de8235cfa17e1b173b6cff6b9a0d681
Summary:
Changelog: [Internal]

TSIA

Differential Revision: https://internalfb.com/D51713088

fbshipit-source-id: 3a2edd39178ad3ae0fac22547302448efc4ea875
…iner for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: 8a90f16e0d67fb2997a6034f2f378dc80c37dc07
@facebook-github-bot
Copy link
Contributor

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

@hoxyq hoxyq force-pushed the export-D51822874 branch from 87a29e9 to d8be3e4 Compare January 2, 2024 12:17
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 4, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: 6e660f226b7ff05146819200692c28454625ea0b
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 5, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: bf0017f7913d8c91fb258b762b00b337ba2895c8
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 8, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: 42d856b60b6dd49decf795b0650a448f5ff5f42c
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 8, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: a41dea76fd7cd59e109bc2a123502bc59d27e8a2
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 11, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: 4a0076224e4e687d271650a894636546fe4569fe
hoxyq pushed a commit to hoxyq/react-native that referenced this pull request Jan 15, 2024
…t container for legacy implementations (facebook#41843)

Summary:
Pull Request resolved: facebook#41843

Changelog:
[General] [Fixed] - inspected elements from React DevTools are now correctly highlighted on a relevant surfaces

For cases when DOM Node APIs are not available (Paper or Fabric without these APIs), we will use newly added `isChildPublicInstance` from renderer.

Similarly to D51713089, this updates implementations to highlight elements only on a single AppContainer.

Differential Revision: D51822874

fbshipit-source-id: 6195f3f7fcf0cbbfaad5588d49370618cf6af4fe
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 15, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 9d846f4.

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