Skip to content

Commit fa1a326

Browse files
author
Brian Vaughn
authored
Update useEditableValue hook to sync external value changes (#16878)
* Update useEditableValue to mirror value cahnges Previously, the hook initialized local state (in useState) to mirror the prop/state value. Updates to the value were ignored though. (Once the state was initialized, it was never updated.) The new hook updates the local/editable state to mirror the external value unless there are already pending, local edits being made. * Optimistic CHANGELOG update * Added additional useEditableValue() unit test cases
1 parent 57bf275 commit fa1a326

File tree

8 files changed

+295
-83
lines changed

8 files changed

+295
-83
lines changed

‎packages/react-devtools-shared/src/__tests__/useEditableValue-test.js‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
describe('useEditableValue', () => {
11+
let act;
12+
let React;
13+
let ReactDOM;
14+
let useEditableValue;
15+
16+
beforeEach(() => {
17+
const utils = require('./utils');
18+
act = utils.act;
19+
20+
React = require('react');
21+
ReactDOM = require('react-dom');
22+
23+
useEditableValue = require('../devtools/views/hooks').useEditableValue;
24+
});
25+
26+
it('should override editable state when external props are updated', () => {
27+
let state;
28+
29+
function Example({value}) {
30+
const tuple = useEditableValue(value);
31+
state = tuple[0];
32+
return null;
33+
}
34+
35+
const container = document.createElement('div');
36+
ReactDOM.render(<Example value={1} />, container);
37+
expect(state.editableValue).toEqual('1');
38+
expect(state.externalValue).toEqual(1);
39+
expect(state.parsedValue).toEqual(1);
40+
expect(state.hasPendingChanges).toBe(false);
41+
expect(state.isValid).toBe(true);
42+
43+
// If there are NO pending changes,
44+
// an update to the external prop value should override the local/pending value.
45+
ReactDOM.render(<Example value={2} />, container);
46+
expect(state.editableValue).toEqual('2');
47+
expect(state.externalValue).toEqual(2);
48+
expect(state.parsedValue).toEqual(2);
49+
expect(state.hasPendingChanges).toBe(false);
50+
expect(state.isValid).toBe(true);
51+
});
52+
53+
it('should not override editable state when external props are updated if there are pending changes', () => {
54+
let dispatch, state;
55+
56+
function Example({value}) {
57+
const tuple = useEditableValue(value);
58+
state = tuple[0];
59+
dispatch = tuple[1];
60+
return null;
61+
}
62+
63+
const container = document.createElement('div');
64+
ReactDOM.render(<Example value={1} />, container);
65+
expect(state.editableValue).toEqual('1');
66+
expect(state.externalValue).toEqual(1);
67+
expect(state.parsedValue).toEqual(1);
68+
expect(state.hasPendingChanges).toBe(false);
69+
expect(state.isValid).toBe(true);
70+
71+
// Update (local) editable state.
72+
act(() =>
73+
dispatch({
74+
type: 'UPDATE',
75+
editableValue: '2',
76+
externalValue: 1,
77+
}),
78+
);
79+
expect(state.editableValue).toEqual('2');
80+
expect(state.externalValue).toEqual(1);
81+
expect(state.parsedValue).toEqual(2);
82+
expect(state.hasPendingChanges).toBe(true);
83+
expect(state.isValid).toBe(true);
84+
85+
// If there ARE pending changes,
86+
// an update to the external prop value should NOT override the local/pending value.
87+
ReactDOM.render(<Example value={3} />, container);
88+
expect(state.editableValue).toEqual('2');
89+
expect(state.externalValue).toEqual(3);
90+
expect(state.parsedValue).toEqual(2);
91+
expect(state.hasPendingChanges).toBe(true);
92+
expect(state.isValid).toBe(true);
93+
});
94+
95+
it('should parse edits to ensure valid JSON', () => {
96+
let dispatch, state;
97+
98+
function Example({value}) {
99+
const tuple = useEditableValue(value);
100+
state = tuple[0];
101+
dispatch = tuple[1];
102+
return null;
103+
}
104+
105+
const container = document.createElement('div');
106+
ReactDOM.render(<Example value={1} />, container);
107+
expect(state.editableValue).toEqual('1');
108+
expect(state.externalValue).toEqual(1);
109+
expect(state.parsedValue).toEqual(1);
110+
expect(state.hasPendingChanges).toBe(false);
111+
expect(state.isValid).toBe(true);
112+
113+
// Update (local) editable state.
114+
act(() =>
115+
dispatch({
116+
type: 'UPDATE',
117+
editableValue: '"a',
118+
externalValue: 1,
119+
}),
120+
);
121+
expect(state.editableValue).toEqual('"a');
122+
expect(state.externalValue).toEqual(1);
123+
expect(state.parsedValue).toEqual(1);
124+
expect(state.hasPendingChanges).toBe(true);
125+
expect(state.isValid).toBe(false);
126+
});
127+
128+
it('should reset to external value upon request', () => {
129+
let dispatch, state;
130+
131+
function Example({value}) {
132+
const tuple = useEditableValue(value);
133+
state = tuple[0];
134+
dispatch = tuple[1];
135+
return null;
136+
}
137+
138+
const container = document.createElement('div');
139+
ReactDOM.render(<Example value={1} />, container);
140+
expect(state.editableValue).toEqual('1');
141+
expect(state.externalValue).toEqual(1);
142+
expect(state.parsedValue).toEqual(1);
143+
expect(state.hasPendingChanges).toBe(false);
144+
expect(state.isValid).toBe(true);
145+
146+
// Update (local) editable state.
147+
act(() =>
148+
dispatch({
149+
type: 'UPDATE',
150+
editableValue: '2',
151+
externalValue: 1,
152+
}),
153+
);
154+
expect(state.editableValue).toEqual('2');
155+
expect(state.externalValue).toEqual(1);
156+
expect(state.parsedValue).toEqual(2);
157+
expect(state.hasPendingChanges).toBe(true);
158+
expect(state.isValid).toBe(true);
159+
160+
// Reset editable state
161+
act(() =>
162+
dispatch({
163+
type: 'RESET',
164+
externalValue: 1,
165+
}),
166+
);
167+
expect(state.editableValue).toEqual('1');
168+
expect(state.externalValue).toEqual(1);
169+
expect(state.parsedValue).toEqual(1);
170+
expect(state.hasPendingChanges).toBe(false);
171+
expect(state.isValid).toBe(true);
172+
});
173+
});

‎packages/react-devtools-shared/src/devtools/views/Components/EditableValue.js‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @flow
88
*/
99

10-
import React, {Fragment, useCallback, useRef} from 'react';
10+
import React, {Fragment, useRef} from 'react';
1111
import Button from '../Button';
1212
import ButtonIcon from '../ButtonIcon';
1313
import styles from './EditableValue.css';
@@ -17,51 +17,51 @@ type OverrideValueFn = (path: Array<string | number>, value: any) => void;
1717

1818
type EditableValueProps = {|
1919
className?: string,
20-
initialValue: any,
2120
overrideValueFn: OverrideValueFn,
2221
path: Array<string | number>,
22+
value: any,
2323
|};
2424

2525
export default function EditableValue({
2626
className = '',
27-
initialValue,
2827
overrideValueFn,
2928
path,
29+
value,
3030
}: EditableValueProps) {
3131
const inputRef = useRef<HTMLInputElement | null>(null);
32-
const {
33-
editableValue,
34-
hasPendingChanges,
35-
isValid,
36-
parsedValue,
37-
reset,
38-
update,
39-
} = useEditableValue(initialValue);
32+
const [state, dispatch] = useEditableValue(value);
33+
const {editableValue, hasPendingChanges, isValid, parsedValue} = state;
4034

41-
const handleChange = useCallback(({target}) => update(target.value), [
42-
update,
43-
]);
35+
const reset = () =>
36+
dispatch({
37+
type: 'RESET',
38+
externalValue: value,
39+
});
4440

45-
const handleKeyDown = useCallback(
46-
event => {
47-
// Prevent keydown events from e.g. change selected element in the tree
48-
event.stopPropagation();
41+
const handleChange = ({target}) =>
42+
dispatch({
43+
type: 'UPDATE',
44+
editableValue: target.value,
45+
externalValue: value,
46+
});
4947

50-
switch (event.key) {
51-
case 'Enter':
52-
if (isValid && hasPendingChanges) {
53-
overrideValueFn(path, parsedValue);
54-
}
55-
break;
56-
case 'Escape':
57-
reset();
58-
break;
59-
default:
60-
break;
61-
}
62-
},
63-
[hasPendingChanges, isValid, overrideValueFn, parsedValue, reset],
64-
);
48+
const handleKeyDown = event => {
49+
// Prevent keydown events from e.g. change selected element in the tree
50+
event.stopPropagation();
51+
52+
switch (event.key) {
53+
case 'Enter':
54+
if (isValid && hasPendingChanges) {
55+
overrideValueFn(path, parsedValue);
56+
}
57+
break;
58+
case 'Escape':
59+
reset();
60+
break;
61+
default:
62+
break;
63+
}
64+
};
6565

6666
let placeholder = '';
6767
if (editableValue === undefined) {

‎packages/react-devtools-shared/src/devtools/views/Components/HooksTree.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,9 @@ function HookView({canEditHooks, hook, id, inspectPath, path}: HookViewProps) {
270270
</span>
271271
{typeof overrideValueFn === 'function' ? (
272272
<EditableValue
273-
initialValue={value}
274273
overrideValueFn={overrideValueFn}
275274
path={[]}
275+
value={value}
276276
/>
277277
) : (
278278
// $FlowFixMe Cannot create span element because in property children

‎packages/react-devtools-shared/src/devtools/views/Components/InspectedElementTree.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ export default function InspectedElementTree({
105105
:&nbsp;
106106
<EditableValue
107107
className={styles.EditableValue}
108-
initialValue={''}
109108
overrideValueFn={handleNewEntryValue}
110109
path={[newPropName]}
110+
value={''}
111111
/>
112112
</div>
113113
)}

‎packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ export default function KeyValue({
102102
</span>
103103
{isEditable ? (
104104
<EditableValue
105-
initialValue={value}
106105
overrideValueFn={((overrideValueFn: any): OverrideValueFn)}
107106
path={path}
107+
value={value}
108108
/>
109109
) : (
110110
<span className={styles.Value}>{displayValue}</span>

‎packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,7 @@ type Props = {|
619619
children: React$Node,
620620

621621
// Used for automated testing
622+
defaultInspectedElementID?: ?number,
622623
defaultOwnerID?: ?number,
623624
defaultSelectedElementID?: ?number,
624625
defaultSelectedElementIndex?: ?number,
@@ -627,6 +628,7 @@ type Props = {|
627628
// TODO Remove TreeContextController wrapper element once global ConsearchText.write API exists.
628629
function TreeContextController({
629630
children,
631+
defaultInspectedElementID,
630632
defaultOwnerID,
631633
defaultSelectedElementID,
632634
defaultSelectedElementIndex,
@@ -700,7 +702,8 @@ function TreeContextController({
700702
ownerFlatTree: null,
701703

702704
// Inspection element panel
703-
inspectedElementID: null,
705+
inspectedElementID:
706+
defaultInspectedElementID == null ? null : defaultInspectedElementID,
704707
});
705708

706709
const dispatchWrapper = useCallback(

0 commit comments

Comments
 (0)