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

feat[devtools]: display Forget badge for the relevant components #27709

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Nov 15, 2023

Adds Forget badge to all relevant components.

Changes:

  • If component is compiled with Forget and using a built-in useMemoCache hook, it will have a Forget badge next to its display name in:
    • components tree
    • inspected element view
    • owners list
  • Such badges are indexable, so Forget components can be searched using search bar.

Fixes:

  • Displaying the badges for owners list inside the inspected component view

Implementation:

  • React DevTools backend is responsible for identifying if component is compiled with Forget, based on fiber.updateQueue.memoCache. It will wrap component's display name with Forget(...) prefix before passing operations to the frontend. On the frontend side, we will parse the display name and strip Forget prefix, marking the corresponding element by setting compiledWithForget field. Almost the same logic is currently used for HOC display names.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 15, 2023
@hoxyq hoxyq changed the title feat[devtools]: display Forget badge for the required components feat[devtools]: display Forget badge for the relevant components Nov 15, 2023
@hoxyq hoxyq force-pushed the devtools/forget-badges branch 2 times, most recently from 403e638 to e4136b8 Compare November 15, 2023 15:34
function getDisplayNameForFiber(fiber: Fiber): string | null {
function getDisplayNameForFiber(
fiber: Fiber,
shouldSkipForgetCheck: boolean = false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to do this without adding a boolean argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably. Without recursion, but with some auxiliary variables. Why do you want to remove it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is unnecessarily complex here with the recursion though? It's harder to reason about

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is unnecessarily complex here with the recursion though?

I don't think so, but I can rewrite it, I don't have a strong opinion on it.

I am kinda surprised that this is the only thing we are discussing in this PR.

Copy link
Contributor

@gsathya gsathya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hoxyq hoxyq merged commit 6c7b41d into facebook:main Nov 23, 2023
36 checks passed
@hoxyq hoxyq deleted the devtools/forget-badges branch November 23, 2023 18:37
hoxyq added a commit that referenced this pull request Nov 29, 2023
### Breaking
* refactor[devtools]: highlight an array of elements for native
([hoxyq](https://github.com/hoxyq) in
[#27734](#27734))

### Features
* feat[devtools]: display Forget badge for the relevant components
([hoxyq](https://github.com/hoxyq) in
[#27709](#27709))

### Other
* Added windows powershell syntax to build scripts
([PrathamLalwani](https://github.com/PrathamLalwani) in
[#27692](#27692))
* refactor[react-devtools-shared]: minor parsing improvements and
modifications ([hoxyq](https://github.com/hoxyq) in
[#27661](#27661))
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…ebook#27709)

Adds `Forget` badge to all relevant components.

Changes:
- If component is compiled with Forget and using a built-in
`useMemoCache` hook, it will have a `Forget` badge next to its display
name in:
  - components tree
  - inspected element view
  - owners list
- Such badges are indexable, so Forget components can be searched using
search bar.

Fixes:
- Displaying the badges for owners list inside the inspected component
view

Implementation:
- React DevTools backend is responsible for identifying if component is
compiled with Forget, based on `fiber.updateQueue.memoCache`. It will
wrap component's display name with `Forget(...)` prefix before passing
operations to the frontend. On the frontend side, we will parse the
display name and strip Forget prefix, marking the corresponding element
by setting `compiledWithForget` field. Almost the same logic is
currently used for HOC display names.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
### Breaking
* refactor[devtools]: highlight an array of elements for native
([hoxyq](https://github.com/hoxyq) in
[facebook#27734](facebook#27734))

### Features
* feat[devtools]: display Forget badge for the relevant components
([hoxyq](https://github.com/hoxyq) in
[facebook#27709](facebook#27709))

### Other
* Added windows powershell syntax to build scripts
([PrathamLalwani](https://github.com/PrathamLalwani) in
[facebook#27692](facebook#27692))
* refactor[react-devtools-shared]: minor parsing improvements and
modifications ([hoxyq](https://github.com/hoxyq) in
[facebook#27661](facebook#27661))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
)

Adds `Forget` badge to all relevant components.

Changes:
- If component is compiled with Forget and using a built-in
`useMemoCache` hook, it will have a `Forget` badge next to its display
name in:
  - components tree
  - inspected element view
  - owners list
- Such badges are indexable, so Forget components can be searched using
search bar.

Fixes:
- Displaying the badges for owners list inside the inspected component
view

Implementation:
- React DevTools backend is responsible for identifying if component is
compiled with Forget, based on `fiber.updateQueue.memoCache`. It will
wrap component's display name with `Forget(...)` prefix before passing
operations to the frontend. On the frontend side, we will parse the
display name and strip Forget prefix, marking the corresponding element
by setting `compiledWithForget` field. Almost the same logic is
currently used for HOC display names.

DiffTrain build for commit 6c7b41d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants