Skip to content

DevTools: Refactor Profiler supported status and UI #23157

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

Closed

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jan 20, 2022

Builds on top of #23141.

This PR makes a few incremental changes to the store and profiler UI:

  • Backend reports legacy profiling support and timeline profiling support separately to the frontend.
  • Store repots static and dynamic/per-root profiling capabilities separately now. (This was previously kind of a bug.)
  • Timeline and legacy profiler buttons ("import data" and "clear data") are now no longer tab specific.

This PR adds the following commits:

  • 17a3820: Main changes explained.
  • 17969bd: Update test (mostly snapshot data)

Related to #22529.


UI changes

Timeline: No data

Timeline: No data

Timeline: Not supported

Timeline: Not supported

Timeline: Not supported (Facebook internal build)

Timeline: Not supported (Facebook internal build)


Note that this PR (in isolation) makes the Timeline profiler feature unusable. It will be part of a stack of PRs that will be merged together once finished.

Brian Vaughn added 4 commits January 20, 2022 13:51
React will call the DevTools profiling hooks unconditionally (for DEV and profiling builds) but DevTools will only log the data to the User Timing API when DevTools is profiling.
@bvaughn bvaughn requested a review from lunaruan January 20, 2022 19:02
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 20, 2022
@sizebot
Copy link

sizebot commented Jan 20, 2022

Comparing: 505c15c...1d6352c

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 129.59 kB 129.59 kB = 41.55 kB 41.55 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 134.77 kB 134.77 kB = 43.10 kB 43.10 kB
facebook-www/ReactDOM-prod.classic.js = 428.19 kB 428.19 kB = 78.60 kB 78.60 kB
facebook-www/ReactDOM-prod.modern.js = 418.18 kB 418.18 kB = 77.17 kB 77.17 kB
facebook-www/ReactDOMForked-prod.classic.js = 428.19 kB 428.19 kB = 78.60 kB 78.60 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 1d6352c

@bvaughn bvaughn force-pushed the devtools-profiler-refactor-phase-4 branch from 17969bd to 1c94a1c Compare January 20, 2022 19:12
Brian Vaughn added 7 commits January 20, 2022 14:40
React will call the DevTools profiling hooks unconditionally (for DEV and profiling builds) but DevTools will only log the data to the User Timing API when DevTools is profiling.
1. Backend reports legacy profiling support and timeline profiling support separately to the frontend.
2. Store repots static and dynamic/per-root profiling capabilities separately now. (This was previously kind of a bug.)
3. Timeline and legacy profiler buttons ("import data" and "clear data") are now no longer tab specific.
@bvaughn bvaughn force-pushed the devtools-profiler-refactor-phase-4 branch from 1c94a1c to 301fe89 Compare January 20, 2022 19:43
@bvaughn bvaughn changed the title Devtools profiler refactor phase 4 DevTools: Refactor Profiler supported status and UI Jan 20, 2022
This was accidentally deleted in the previous commit.
Copy link
Contributor

@lunaruan lunaruan left a comment

Choose a reason for hiding this comment

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

Approved pending changes we talked about (deleting the accidental code commit and the rename of legacy etc. to basic support!)

@bvaughn
Copy link
Contributor Author

bvaughn commented Jan 20, 2022

Thank you for the review! I'm going to squash all of these PRs and commits here:
https://github.com/facebook/react/compare/main...bvaughn:devtools-timeline-profiler-squashed?expand=1

I'll based my next branch off of that :)

@bvaughn
Copy link
Contributor Author

bvaughn commented Jan 20, 2022

Squashed into #23158

@bvaughn bvaughn closed this Jan 20, 2022
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