DevTools: refactor NativeStyleEditor, don't use custom cache implementation (#32298)
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 - https://github.com/facebook/react/pull/30555. Looks like I've introduced a regression in https://github.com/facebook/react/pull/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.
Ruslan Lesiutin committed
Feb 5, 2025 at 12:52 UTC
d85cf3e5ab6e049626a8bedddffbaec05c516195
8 files changed
+67
-172
packages/react-devtools-inline/__tests__/__e2e__/components.test.js
+14
-20
@@ -212,8 +212,8 @@ test.describe('Components', () => {
212
});
213
214
test('should allow searching for component by name', async () => {
215
- async function getComponentSearchResultsCount() {
216
- return await page.evaluate(() => {
215
+ async function waitForComponentSearchResultsCount(text) {
216
+ return await page.waitForFunction(expectedElementText => {
217
const {createTestNameSelector, findAllNodes} =
218
window.REACT_DOM_DEVTOOLS;
219
const container = document.getElementById('devtools');
@@ -221,8 +221,10 @@ test.describe('Components', () => {
221
const element = findAllNodes(container, [
222
createTestNameSelector('ComponentSearchInput-ResultsCount'),
223
])[0];
224
- return element.innerText;
225
- });
224
+ return element !== undefined
225
+ ? element.innerText === expectedElementText
226
+ : false;
227
+ }, text);
228
}
229
230
async function focusComponentSearch() {
@@ -238,35 +240,27 @@ test.describe('Components', () => {
240
241
await focusComponentSearch();
242
await page.keyboard.insertText('List');
241
- let count = await getComponentSearchResultsCount();
242
- expect(count).toBe('1 | 4');
243
+ await waitForComponentSearchResultsCount('1 | 4');
244
245
await page.keyboard.insertText('Item');
245
- count = await getComponentSearchResultsCount();
246
- expect(count).toBe('1 | 3');
246
+ await waitForComponentSearchResultsCount('1 | 3');
247
248
await page.keyboard.press('Enter');
249
- count = await getComponentSearchResultsCount();
250
- expect(count).toBe('2 | 3');
249
+ await waitForComponentSearchResultsCount('2 | 3');
250
251
await page.keyboard.press('Enter');
253
- count = await getComponentSearchResultsCount();
254
- expect(count).toBe('3 | 3');
252
+ await waitForComponentSearchResultsCount('3 | 3');
253
254
await page.keyboard.press('Enter');
257
- count = await getComponentSearchResultsCount();
258
- expect(count).toBe('1 | 3');
255
+ await waitForComponentSearchResultsCount('1 | 3');
256
257
await page.keyboard.press('Shift+Enter');
261
- count = await getComponentSearchResultsCount();
262
- expect(count).toBe('3 | 3');
258
+ await waitForComponentSearchResultsCount('3 | 3');
259
260
await page.keyboard.press('Shift+Enter');
265
- count = await getComponentSearchResultsCount();
266
- expect(count).toBe('2 | 3');
261
+ await waitForComponentSearchResultsCount('2 | 3');
262
263
await page.keyboard.press('Shift+Enter');
269
- count = await getComponentSearchResultsCount();
270
- expect(count).toBe('1 | 3');
264
+ await waitForComponentSearchResultsCount('1 | 3');
265
});
266
});
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/LayoutViewer.css
-1
@@ -1,6 +1,5 @@
1
.LayoutViewer {
2
padding: 0.25rem;
3
- border-top: 1px solid var(--color-border);
3
font-family: var(--font-family-monospace);
4
font-size: var(--font-size-monospace-small);
5
}
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/StyleEditor.css
-1
@@ -2,7 +2,6 @@
2
font-family: var(--font-family-monospace);
3
font-size: var(--font-size-monospace-normal);
4
padding: 0.25rem;
5
- border-top: 1px solid var(--color-border);
5
}
6
7
.HeaderRow {
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/StyleEditor.js
+1
-1
@@ -81,7 +81,7 @@ export default function StyleEditor({id, style}: Props): React.Node {
81
{keys.length > 0 &&
82
keys.map(attribute => (
83
<Row
84
- key={attribute}
84
+ key={`${attribute}/${style[attribute]}`}
85
attribute={attribute}
86
changeAttribute={changeAttribute}
87
changeValue={changeValue}
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js
+21
-121
@@ -10,75 +10,26 @@
10
import type {ReactContext} from 'shared/ReactTypes';
11
12
import * as React from 'react';
13
-import {
14
- createContext,
15
- useCallback,
16
- useContext,
17
- useEffect,
18
- useMemo,
19
- useState,
20
-} from 'react';
21
-import {createResource} from 'react-devtools-shared/src/devtools/cache';
13
+import {createContext, useContext, useEffect, useState} from 'react';
14
import {
15
BridgeContext,
16
StoreContext,
17
} from 'react-devtools-shared/src/devtools/views/context';
26
-import {TreeStateContext} from '../TreeContext';
18
+import {TreeStateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
19
28
-import type {StateContext} from '../TreeContext';
20
+import type {StateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
21
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
22
import type Store from 'react-devtools-shared/src/devtools/store';
23
import type {StyleAndLayout as StyleAndLayoutBackend} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';
24
import type {StyleAndLayout as StyleAndLayoutFrontend} from './types';
33
-import type {Element} from 'react-devtools-shared/src/frontend/types';
34
-import type {
35
- Resource,
36
- Thenable,
37
-} from 'react-devtools-shared/src/devtools/cache';
38
-
39
-export type GetStyleAndLayout = (id: number) => StyleAndLayoutFrontend | null;
25
41
-type Context = {
42
- getStyleAndLayout: GetStyleAndLayout,
43
-};
26
+type Context = StyleAndLayoutFrontend | null;
27
28
const NativeStyleContext: ReactContext<Context> = createContext<Context>(
29
((null: any): Context),
30
);
31
NativeStyleContext.displayName = 'NativeStyleContext';
32
50
-type ResolveFn = (styleAndLayout: StyleAndLayoutFrontend) => void;
51
-type InProgressRequest = {
52
- promise: Thenable<StyleAndLayoutFrontend>,
53
- resolveFn: ResolveFn,
54
-};
55
-
56
-const inProgressRequests: WeakMap<Element, InProgressRequest> = new WeakMap();
57
-const resource: Resource<Element, Element, StyleAndLayoutFrontend> =
58
- createResource(
59
- (element: Element) => {
60
- const request = inProgressRequests.get(element);
61
- if (request != null) {
62
- return request.promise;
63
- }
64
-
65
- let resolveFn:
66
- | ResolveFn
67
- | ((
68
- result: Promise<StyleAndLayoutFrontend> | StyleAndLayoutFrontend,
69
- ) => void) = ((null: any): ResolveFn);
70
- const promise = new Promise(resolve => {
71
- resolveFn = resolve;
72
- });
73
-
74
- inProgressRequests.set(element, ({promise, resolveFn}: $FlowFixMe));
75
-
76
- return (promise: $FlowFixMe);
77
- },
78
- (element: Element) => element,
79
- {useWeakMap: true},
80
- );
81
-
33
type Props = {
34
children: React$Node,
35
};
@@ -86,72 +37,22 @@ type Props = {
37
function NativeStyleContextController({children}: Props): React.Node {
38
const bridge = useContext<FrontendBridge>(BridgeContext);
39
const store = useContext<Store>(StoreContext);
89
-
90
- const getStyleAndLayout = useCallback<GetStyleAndLayout>(
91
- (id: number) => {
92
- const element = store.getElementByID(id);
93
- if (element !== null) {
94
- return resource.read(element);
95
- } else {
96
- return null;
97
- }
98
- },
99
- [store],
100
- );
101
-
102
- // It's very important that this context consumes inspectedElementID and not NativeStyleID.
103
- // Otherwise the effect that sends the "inspect" message across the bridge-
104
- // would itself be blocked by the same render that suspends (waiting for the data).
40
const {inspectedElementID} = useContext<StateContext>(TreeStateContext);
41
42
const [currentStyleAndLayout, setCurrentStyleAndLayout] =
43
useState<StyleAndLayoutFrontend | null>(null);
44
110
- // This effect handler invalidates the suspense cache and schedules rendering updates with React.
111
- useEffect(() => {
112
- const onStyleAndLayout = ({id, layout, style}: StyleAndLayoutBackend) => {
113
- const element = store.getElementByID(id);
114
- if (element !== null) {
115
- const styleAndLayout: StyleAndLayoutFrontend = {
116
- layout,
117
- style,
118
- };
119
- const request = inProgressRequests.get(element);
120
- if (request != null) {
121
- inProgressRequests.delete(element);
122
- request.resolveFn(styleAndLayout);
123
- setCurrentStyleAndLayout(styleAndLayout);
124
- } else {
125
- resource.write(element, styleAndLayout);
126
-
127
- // Schedule update with React if the currently-selected element has been invalidated.
128
- if (id === inspectedElementID) {
129
- setCurrentStyleAndLayout(styleAndLayout);
130
- }
131
- }
132
- }
133
- };
134
-
135
- bridge.addListener('NativeStyleEditor_styleAndLayout', onStyleAndLayout);
136
- return () =>
137
- bridge.removeListener(
138
- 'NativeStyleEditor_styleAndLayout',
139
- onStyleAndLayout,
140
- );
141
- }, [bridge, currentStyleAndLayout, inspectedElementID, store]);
142
-
45
// This effect handler polls for updates on the currently selected element.
46
useEffect(() => {
47
if (inspectedElementID === null) {
48
+ setCurrentStyleAndLayout(null);
49
return () => {};
50
}
51
149
- const rendererID = store.getRendererIDForElement(inspectedElementID);
150
-
151
- let timeoutID: TimeoutID | null = null;
152
-
52
+ let requestTimeoutId: TimeoutID | null = null;
53
const sendRequest = () => {
154
- timeoutID = null;
54
+ requestTimeoutId = null;
55
+ const rendererID = store.getRendererIDForElement(inspectedElementID);
56
57
if (rendererID !== null) {
58
bridge.send('NativeStyleEditor_measure', {
@@ -165,38 +66,37 @@ function NativeStyleContextController({children}: Props): React.Node {
66
// We'll poll for an update in the response handler below.
67
sendRequest();
68
168
- const onStyleAndLayout = ({id}: StyleAndLayoutBackend) => {
69
+ const onStyleAndLayout = ({id, layout, style}: StyleAndLayoutBackend) => {
70
// If this is the element we requested, wait a little bit and then ask for another update.
71
if (id === inspectedElementID) {
171
- if (timeoutID !== null) {
172
- clearTimeout(timeoutID);
72
+ if (requestTimeoutId !== null) {
73
+ clearTimeout(requestTimeoutId);
74
}
174
- timeoutID = setTimeout(sendRequest, 1000);
75
+ requestTimeoutId = setTimeout(sendRequest, 1000);
76
}
77
+
78
+ const styleAndLayout: StyleAndLayoutFrontend = {
79
+ layout,
80
+ style,
81
+ };
82
+ setCurrentStyleAndLayout(styleAndLayout);
83
};
84
85
bridge.addListener('NativeStyleEditor_styleAndLayout', onStyleAndLayout);
179
-
86
return () => {
87
bridge.removeListener(
88
'NativeStyleEditor_styleAndLayout',
89
onStyleAndLayout,
90
);
91
186
- if (timeoutID !== null) {
187
- clearTimeout(timeoutID);
92
+ if (requestTimeoutId !== null) {
93
+ clearTimeout(requestTimeoutId);
94
}
95
};
96
}, [bridge, inspectedElementID, store]);
97
192
- const value = useMemo(
193
- () => ({getStyleAndLayout}),
194
- // NativeStyle is used to invalidate the cache and schedule an update with React.
195
- [currentStyleAndLayout, getStyleAndLayout],
196
- );
197
-
98
return (
199
- <NativeStyleContext.Provider value={value}>
99
+ <NativeStyleContext.Provider value={currentStyleAndLayout}>
100
{children}
101
</NativeStyleContext.Provider>
102
);
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/index.css
new
+3
@@ -0,0 +1,3 @@
1
+.Stack > *:not(:first-child) {
2
+ border-top: 1px solid var(--color-border);
3
+}
packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/index.js
+17
-22
@@ -8,19 +8,19 @@
8
*/
9
10
import * as React from 'react';
11
-import {Fragment, useContext, useMemo} from 'react';
11
+import {useContext, useMemo} from 'react';
12
+
13
import {StoreContext} from 'react-devtools-shared/src/devtools/views/context';
14
import {useSubscription} from 'react-devtools-shared/src/devtools/views/hooks';
15
+import {TreeStateContext} from 'react-devtools-shared/src/devtools/views/Components/TreeContext';
16
+
17
import {NativeStyleContext} from './context';
18
import LayoutViewer from './LayoutViewer';
19
import StyleEditor from './StyleEditor';
17
-import {TreeStateContext} from '../TreeContext';
18
-
19
-type Props = {};
20
+import styles from './index.css';
21
21
-export default function NativeStyleEditorWrapper(_: Props): React.Node {
22
+export default function NativeStyleEditorWrapper(): React.Node {
23
const store = useContext(StoreContext);
23
-
24
const subscription = useMemo(
25
() => ({
26
getCurrentValue: () => store.supportsNativeStyleEditor,
@@ -33,8 +33,8 @@ export default function NativeStyleEditorWrapper(_: Props): React.Node {
33
}),
34
[store],
35
);
36
- const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
36
37
+ const supportsNativeStyleEditor = useSubscription<boolean>(subscription);
38
if (!supportsNativeStyleEditor) {
39
return null;
40
}
@@ -42,32 +42,27 @@ export default function NativeStyleEditorWrapper(_: Props): React.Node {
42
return <NativeStyleEditor />;
43
}
44
45
-function NativeStyleEditor(_: Props) {
46
- const {getStyleAndLayout} = useContext(NativeStyleContext);
47
-
45
+function NativeStyleEditor() {
46
const {inspectedElementID} = useContext(TreeStateContext);
47
+ const inspectedElementStyleAndLayout = useContext(NativeStyleContext);
48
if (inspectedElementID === null) {
49
return null;
50
}
52
-
53
- const maybeStyleAndLayout = getStyleAndLayout(inspectedElementID);
54
- if (maybeStyleAndLayout === null) {
51
+ if (inspectedElementStyleAndLayout === null) {
52
return null;
53
}
54
58
- const {layout, style} = maybeStyleAndLayout;
55
+ const {layout, style} = inspectedElementStyleAndLayout;
56
+ if (layout === null && style === null) {
57
+ return null;
58
+ }
59
60
return (
61
- <Fragment>
61
+ <div className={styles.Stack}>
62
{layout !== null && (
63
<LayoutViewer id={inspectedElementID} layout={layout} />
64
)}
65
- {style !== null && (
66
- <StyleEditor
67
- id={inspectedElementID}
68
- style={style !== null ? style : {}}
69
- />
70
- )}
71
- </Fragment>
65
+ {style !== null && <StyleEditor id={inspectedElementID} style={style} />}
66
+ </div>
67
);
68
}
packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js
+11
-6
@@ -35,6 +35,7 @@ import {
35
useMemo,
36
useReducer,
37
useRef,
38
+ startTransition,
39
} from 'react';
40
import {createRegExp} from '../utils';
41
import {StoreContext} from '../context';
@@ -890,15 +891,19 @@ function TreeContextController({
891
? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId)
892
: null,
893
});
894
+ const dispatchWrapper = useMemo(
895
+ () => (action: Action) => startTransition(() => dispatch(action)),
896
+ [dispatch],
897
+ );
898
899
// Listen for host element selections.
900
useEffect(() => {
901
const handler = (id: Element['id']) =>
897
- dispatch({type: 'SELECT_ELEMENT_BY_ID', payload: id});
902
+ dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id});
903
904
store.addListener('hostInstanceSelected', handler);
905
return () => store.removeListener('hostInstanceSelected', handler);
901
- }, [store, dispatch]);
906
+ }, [store, dispatchWrapper]);
907
908
// If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it.
909
// This needs to be a layout effect to avoid temporarily flashing an incorrect selection.
@@ -922,7 +927,7 @@ function TreeContextController({
927
Array<number>,
928
Map<number, number>,
929
]) => {
925
- dispatch({
930
+ dispatchWrapper({
931
type: 'HANDLE_STORE_MUTATION',
932
payload: [addedElementIDs, removedElementIDs],
933
});
@@ -933,7 +938,7 @@ function TreeContextController({
938
// At the moment, we can treat this as a mutation.
939
// We don't know which Elements were newly added/removed, but that should be okay in this case.
940
// It would only impact the search state, which is unlikely to exist yet at this point.
936
- dispatch({
941
+ dispatchWrapper({
942
type: 'HANDLE_STORE_MUTATION',
943
payload: [[], new Map()],
944
});
@@ -941,11 +946,11 @@ function TreeContextController({
946
947
store.addListener('mutated', handleStoreMutated);
948
return () => store.removeListener('mutated', handleStoreMutated);
944
- }, [dispatch, initialRevision, store]);
949
+ }, [dispatchWrapper, initialRevision, store]);
950
951
return (
952
<TreeStateContext.Provider value={state}>
948
- <TreeDispatcherContext.Provider value={dispatch}>
953
+ <TreeDispatcherContext.Provider value={dispatchWrapper}>
954
{children}
955
</TreeDispatcherContext.Provider>
956
</TreeStateContext.Provider>