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

DevTools: refactor NativeStyleEditor, don't use custom cache implementation #32298

Merged

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Feb 3, 2025

We have this really old (5+ years) feature for inspecting native styles of React Native Host components.

We also have a custom Cache implementation in React DevTools, which was forked from React at some point. We know that this should be removed, but it spans through critical parts of the application, like fetching and caching inspected element.

Before this PR, this was also used for caching native style and layouts of RN Host components. This approach is out of date, and was based on the presence of Suspense boundary around inspected element View, which we have removed to speed up element inspection - #30555.

Looks like I've introduced a regression in #31956:

  • Custom Cache implementation will throw thenables and suspend.
  • Because of this, some descendant Suspense boundaries will not resolve for a long time, and React will throw an error https://react.dev/errors/482.

I've switched from a usage of this custom Cache implementation to a naive fetching in effect and keeping the layout and style in a local state of a Context, which will be propagated downwards. The race should be impossible, this is guaranteed by the mechanism for queueing messages through microtasks queue.

The only downside is the UI. If you quickly switch between 2 elements, and one of them has native style, while the other doesn't, UI will feel jumpy. We can address this later with a Suspense boundary, if needed.

@hoxyq hoxyq requested review from vzaidman and eps1lon February 3, 2025 09:58
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Feb 3, 2025
@hoxyq hoxyq force-pushed the react-devtools/refactor-native-style-editor branch from f019f24 to 87b8b4e Compare February 3, 2025 11:13
@Aatif274

This comment was marked as spam.

Copy link
Member

@huntie huntie left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@hoxyq hoxyq merged commit d85cf3e into facebook:main Feb 5, 2025
191 checks passed
@hoxyq hoxyq deleted the react-devtools/refactor-native-style-editor branch February 5, 2025 12:52
hoxyq added a commit that referenced this pull request Feb 7, 2025
Full list of changes:
* DevTools: refactor NativeStyleEditor, don't use custom cache
implementation ([hoxyq](https://github.com/hoxyq) in
[#32298](#32298))
* fix[react-devtools-fusebox]: add extension globals to build
([hoxyq](https://github.com/hoxyq) in
[#32297](#32297))
* DevTools: fix host component filter option title
([hoxyq](https://github.com/hoxyq) in
[#32296](#32296))
* chore[DevTools]: make clipboardWrite optional for chromium
([hoxyq](https://github.com/hoxyq) in
[#32262](#32262))
* DevTools: support useEffectEvent and forward-fix experimental prefix
support ([hoxyq](https://github.com/hoxyq) in
[#32106](#32106))
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