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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Nov 23, 2023
Merged

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

merged 1 commit into from
Nov 23, 2023

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Nov 15, 2023 •
edited
Loading

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
poteto
poteto reviewed Nov 15, 2023
View reviewed changes
packages/react-devtools-shared/src/backend/renderer.js
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.

gsathya
gsathya approved these changes Nov 22, 2023
View reviewed changes
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 hoxyq mentioned this pull request Nov 29, 2023
Merged
hoxyq added a commit that referenced this pull request Nov 29, 2023
@hoxyq
React DevTools 4.28.5 -> 5.0.0 (#27759)
87cb0bf 
### 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))
@ztanner ztanner mentioned this pull request Dec 15, 2023
Merged
ztanner added a commit to vercel/next.js that referenced this pull request Dec 15, 2023
@ztanner
Update React from 2c338b16f to 0cdfef19b (#59666)
1fa7a74 
Updates React from 2c338b16f to 0cdfef19b.

### React upstream changes

- facebook/react#27821
- facebook/react#27820
- facebook/react#27818
- facebook/react#27819
- facebook/react#27817
- facebook/react#27703
- facebook/react#27796
- facebook/react#27811
- facebook/react#27804
- facebook/react#27807
- facebook/react#27805
- facebook/react#27792
- facebook/react#27788
- facebook/react#26852
- facebook/react#27790
- facebook/react#27786
- facebook/react#27785
- facebook/react#27783
- facebook/react#27784
- facebook/react#27769
- facebook/react#27766
- facebook/react#27701
- facebook/react#27732
- facebook/react#27740
- facebook/react#27767
- facebook/react#27768
- facebook/react#27765
- facebook/react#27759
- facebook/react#27579
- facebook/react#27709
- facebook/react#27734
- facebook/react#27739
- facebook/react#27717
- facebook/react#27583
- facebook/react#27713

Closes NEXT-1887
agustints pushed a commit to agustints/next.js that referenced this pull request Jan 6, 2024
@ztanner @agustints
Update React from 2c338b16f to 0cdfef19b (vercel#59666)
9620d19 
Updates React from 2c338b16f to 0cdfef19b.

### React upstream changes

- facebook/react#27821
- facebook/react#27820
- facebook/react#27818
- facebook/react#27819
- facebook/react#27817
- facebook/react#27703
- facebook/react#27796
- facebook/react#27811
- facebook/react#27804
- facebook/react#27807
- facebook/react#27805
- facebook/react#27792
- facebook/react#27788
- facebook/react#26852
- facebook/react#27790
- facebook/react#27786
- facebook/react#27785
- facebook/react#27783
- facebook/react#27784
- facebook/react#27769
- facebook/react#27766
- facebook/react#27701
- facebook/react#27732
- facebook/react#27740
- facebook/react#27767
- facebook/react#27768
- facebook/react#27765
- facebook/react#27759
- facebook/react#27579
- facebook/react#27709
- facebook/react#27734
- facebook/react#27739
- facebook/react#27717
- facebook/react#27583
- facebook/react#27713

Closes NEXT-1887
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@hoxyq @AndyPengc12
feat[devtools]: display Forget badge for the relevant components (fac…
035c9e7 
…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
@hoxyq @AndyPengc12
React DevTools 4.28.5 -> 5.0.0 (facebook#27759)
215b70f 
### 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
@hoxyq
feat[devtools]: display Forget badge for the relevant components (#27709
b16431a 
)

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
Reviewers

@poteto poteto poteto left review comments

@gsathya gsathya gsathya approved these changes

@mofeiZ mofeiZ Awaiting requested review from mofeiZ

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hoxyq @gsathya @poteto @facebook-github-bot