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

Split React DevTools into individual panels #107

Merged
merged 2 commits into from
Aug 30, 2024

Conversation

huntie
Copy link

@huntie huntie commented Aug 29, 2024

Summary

Split the previous React DevTools panel into two individual panels, matching the current Chrome extension on web.

  front_end/panels/react_devtools/
  ├── BUILD.gn
+ ├── ReactDevToolsComponentsView.ts
  ├── ReactDevToolsModel.ts
+ ├── ReactDevToolsProfilerView.ts
+ ├── ReactDevToolsViewBase.ts
  ├── react_devtools.ts
+ ├── react_devtools_components-meta.ts
+ └── react_devtools_profiler-meta.ts

Key changes:

  • Updates React DevTools artifacts to be based on [DevTools] Separate RDT Fusebox into single-panel entry points facebook/react#30708 (stacked from React DevTools 5.3.0 (facebook/react@1434af3).
  • Adds separate entry points extending ReactDevToolsViewBase.
  • Moves ownership of wall, bridge, store, listeners into ReactDevToolsModel (to share across each panel).
    • This uses an ensureInitialized/isInitialized pattern to run ReactDevToolsBindingsModel side-effects once.
  • Refactors ReactDevToolsViewBase to use the SDKModelObserver (modelAdded) pattern to reduce boilerplate.

Test plan

image

🎬 https://pxl.cl/5tXQp

✅ ⚛️ Elements panel: Working Element highlighting and mouse hovering/panel focus changes
✅ Switching to another panel and back preserves highlighted element and scroll position (no re-mounting)
✅ ⚛️ Profiler panel: Switching to panel initialises correctly, profile can be taken
✅ Reload behaviour: Both panels reset and load (in reverse order)

Upstreaming plan

  • This commit should be sent as a patch to the upstream devtools-frontend repo. I've reviewed the contribution guide.
  • This commit is React Native-specific and cannot be upstreamed.

@huntie huntie force-pushed the split-rdt-panels-2 branch 3 times, most recently from d13cee8 to f1b2e42 Compare August 29, 2024 15:05
@huntie huntie marked this pull request as ready for review August 29, 2024 15:13
front_end/panels/react_devtools/ReactDevToolsModel.ts Outdated Show resolved Hide resolved
front_end/panels/react_devtools/ReactDevToolsModel.ts Outdated Show resolved Hide resolved
readonly #wall: ReactDevToolsTypes.Wall;
readonly #bindingsModel: ReactNativeModels.ReactDevToolsBindingsModel.ReactDevToolsBindingsModel;
readonly #listeners: Set<ReactDevToolsTypes.WallListener> = new Set();
#initialized: boolean = false;
Copy link

Choose a reason for hiding this comment

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

TypeScript supports private keyword, and it is far more popular than # in this codebase. I don't have a strong opinion on this, but would prefer sticking to a single style across the whole codebase.

Is there a specific reason to use # over private?

Copy link
Author

@huntie huntie Aug 30, 2024

Choose a reason for hiding this comment

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

Yeah, functionally # will act as private members at runtime, added to JS later than TypeScript's private (which will simply do a compile time check). We definitely don't need to replace everywhere, but I'd prefer this convention in RN-unique files.

front_end/panels/react_devtools/ReactDevToolsModel.ts Outdated Show resolved Hide resolved
@huntie huntie merged commit a556d26 into facebookexperimental:main Aug 30, 2024
3 checks passed
@huntie huntie deleted the split-rdt-panels-2 branch August 30, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants