From 52c17d5ea4010f9a9101dcecf1387f034a88dcc4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 24 Sep 2019 11:52:50 -0700 Subject: [PATCH] Update useEditableValue hook to sync external value changes --- .../src/__tests__/jestCssTransform.js | 3 + .../src/__tests__/useEditableValue-test.js | 84 ++++++++++++ .../views/Components/EditableValue.js | 29 ++-- .../devtools/views/Components/HooksTree.js | 2 +- .../views/Components/InspectedElementTree.js | 2 +- .../src/devtools/views/Components/KeyValue.js | 2 +- .../views/Components/SelectedElement.js | 2 + .../devtools/views/Components/TreeContext.js | 5 +- .../src/devtools/views/hooks.js | 126 +++++++++++------- 9 files changed, 192 insertions(+), 63 deletions(-) create mode 100644 packages/react-devtools-shared/src/__tests__/jestCssTransform.js create mode 100644 packages/react-devtools-shared/src/__tests__/useEditableValue-test.js diff --git a/packages/react-devtools-shared/src/__tests__/jestCssTransform.js b/packages/react-devtools-shared/src/__tests__/jestCssTransform.js new file mode 100644 index 0000000000000..e3a78e5320481 --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/jestCssTransform.js @@ -0,0 +1,3 @@ +// This is a custom Jest transformer turning style imports into empty objects. + +module.exports = {}; diff --git a/packages/react-devtools-shared/src/__tests__/useEditableValue-test.js b/packages/react-devtools-shared/src/__tests__/useEditableValue-test.js new file mode 100644 index 0000000000000..84f429f281a10 --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/useEditableValue-test.js @@ -0,0 +1,84 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +describe('useEditableValue', () => { + let act; + let React; + let ReactDOM; + let useEditableValue; + + beforeEach(() => { + const utils = require('./utils'); + act = utils.act; + + React = require('react'); + ReactDOM = require('react-dom'); + + useEditableValue = require('../devtools/views/hooks').useEditableValue; + }); + + it('should override editable state when external props are updated', () => { + let state; + + function Example({value}) { + const tuple = useEditableValue(value); + state = tuple[0]; + return null; + } + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(state.editableValue).toEqual('"foo"'); + expect(state.externalValue).toEqual('foo'); + expect(state.hasPendingChanges).toBe(false); + + // If there are NO pending changes, + // an update to the external prop value should override the local/pending value. + ReactDOM.render(, container); + expect(state.editableValue).toEqual('"bar"'); + expect(state.externalValue).toEqual('bar'); + expect(state.hasPendingChanges).toBe(false); + }); + + it('should not override editable state when external props are updated if there are pending changes', () => { + let dispatch, state; + + function Example({value}) { + const tuple = useEditableValue(value); + state = tuple[0]; + dispatch = tuple[1]; + return null; + } + + const container = document.createElement('div'); + ReactDOM.render(, container); + expect(state.editableValue).toEqual('"foo"'); + expect(state.externalValue).toEqual('foo'); + expect(state.hasPendingChanges).toBe(false); + + // Update (local) editable state. + act(() => + dispatch({ + type: 'UPDATE', + editableValue: 'not-foo', + externalValue: 'foo', + }), + ); + expect(state.editableValue).toEqual('not-foo'); + expect(state.externalValue).toEqual('foo'); + expect(state.hasPendingChanges).toBe(true); + + // If there ARE pending changes, + // an update to the external prop value should NOT override the local/pending value. + ReactDOM.render(, container); + expect(state.editableValue).toEqual('not-foo'); + expect(state.externalValue).toEqual('bar'); + expect(state.hasPendingChanges).toBe(true); + }); +}); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/EditableValue.js b/packages/react-devtools-shared/src/devtools/views/Components/EditableValue.js index a0429bf8d37de..99b762ca961c2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/EditableValue.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/EditableValue.js @@ -17,30 +17,33 @@ type OverrideValueFn = (path: Array, value: any) => void; type EditableValueProps = {| className?: string, - initialValue: any, overrideValueFn: OverrideValueFn, path: Array, + value: any, |}; export default function EditableValue({ className = '', - initialValue, overrideValueFn, path, + value, }: EditableValueProps) { const inputRef = useRef(null); - const { - editableValue, - hasPendingChanges, - isValid, - parsedValue, - reset, - update, - } = useEditableValue(initialValue); + const [state, dispatch] = useEditableValue(value); + const {editableValue, hasPendingChanges, isValid, parsedValue} = state; - const handleChange = useCallback(({target}) => update(target.value), [ - update, - ]); + const reset = () => + dispatch({ + type: 'RESET', + externalValue: value, + }); + + const handleChange = ({target}) => + dispatch({ + type: 'UPDATE', + editableValue: target.value, + externalValue: value, + }); const handleKeyDown = useCallback( event => { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/HooksTree.js b/packages/react-devtools-shared/src/devtools/views/Components/HooksTree.js index ce0be7c47a1ef..b02e8faa95f39 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/HooksTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/HooksTree.js @@ -270,9 +270,9 @@ function HookView({canEditHooks, hook, id, inspectPath, path}: HookViewProps) { {typeof overrideValueFn === 'function' ? ( ) : ( // $FlowFixMe Cannot create span element because in property children diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementTree.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementTree.js index 9c81c3f952fdb..7a5dbae571557 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementTree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementTree.js @@ -105,9 +105,9 @@ export default function InspectedElementTree({ :  )} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js index 9e82eefa426fb..76b1a2e57b897 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js @@ -102,9 +102,9 @@ export default function KeyValue({ {isEditable ? ( ) : ( {displayValue} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/SelectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/SelectedElement.js index 08d15104ec8e5..50da4fc3a6921 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/SelectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/SelectedElement.js @@ -40,6 +40,7 @@ export type Props = {||}; export default function SelectedElement(_: Props) { const {inspectedElementID} = useContext(TreeStateContext); + console.log(' inspectedElementID', inspectedElementID); const dispatch = useContext(TreeDispatcherContext); const {canViewElementSourceFunction, viewElementSourceFunction} = useContext( ViewElementSourceContext, @@ -59,6 +60,7 @@ export default function SelectedElement(_: Props) { const inspectedElement = inspectedElementID != null ? getInspectedElement(inspectedElementID) : null; + console.log(' inspectedElement', inspectedElement); const highlightElement = useCallback( () => { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js index f3f3c19948070..54738bf3d4b71 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js @@ -619,6 +619,7 @@ type Props = {| children: React$Node, // Used for automated testing + defaultInspectedElementID?: ?number, defaultOwnerID?: ?number, defaultSelectedElementID?: ?number, defaultSelectedElementIndex?: ?number, @@ -627,6 +628,7 @@ type Props = {| // TODO Remove TreeContextController wrapper element once global ConsearchText.write API exists. function TreeContextController({ children, + defaultInspectedElementID, defaultOwnerID, defaultSelectedElementID, defaultSelectedElementIndex, @@ -700,7 +702,8 @@ function TreeContextController({ ownerFlatTree: null, // Inspection element panel - inspectedElementID: null, + inspectedElementID: + defaultInspectedElementID == null ? null : defaultInspectedElementID, }); const dispatchWrapper = useCallback( diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index 57ebb69f6e01c..430d2472eeb4a 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -8,68 +8,102 @@ */ import throttle from 'lodash.throttle'; -import {useCallback, useEffect, useLayoutEffect, useState} from 'react'; -import {unstable_batchedUpdates as batchedUpdates} from 'react-dom'; +import { + useCallback, + useEffect, + useLayoutEffect, + useReducer, + useState, +} from 'react'; import { localStorageGetItem, localStorageSetItem, } from 'react-devtools-shared/src/storage'; import {sanitizeForParse, smartParse, smartStringify} from '../utils'; -type EditableValue = {| +type ACTION_RESET = {| + type: 'RESET', + externalValue: any, +|}; +type ACTION_UPDATE = {| + type: 'UPDATE', + editableValue: any, + externalValue: any, +|}; + +type UseEditableValueAction = ACTION_RESET | ACTION_UPDATE; +type UseEditableValueDispatch = (action: UseEditableValueAction) => void; +type UseEditableValueState = {| editableValue: any, + externalValue: any, hasPendingChanges: boolean, isValid: boolean, parsedValue: any, - reset: () => void, - update: (newValue: any) => void, |}; +function useEditableValueReducer(state, action) { + switch (action.type) { + case 'RESET': + return { + ...state, + editableValue: smartStringify(action.externalValue), + externalValue: action.externalValue, + hasPendingChanges: false, + isValid: true, + parsedValue: action.externalValue, + }; + case 'UPDATE': + let isNewValueValid = false; + let newParsedValue; + try { + newParsedValue = smartParse(action.editableValue); + isNewValueValid = true; + } catch (error) {} + return { + ...state, + editableValue: sanitizeForParse(action.editableValue), + externalValue: action.externalValue, + hasPendingChanges: + smartStringify(action.externalValue) !== action.editableValue, + isValid: isNewValueValid, + parsedValue: isNewValueValid ? newParsedValue : state.parsedValue, + }; + default: + throw new Error(`Invalid action "${action.type}"`); + } +} + // Convenience hook for working with an editable value that is validated via JSON.parse. export function useEditableValue( - initialValue: any, - initialIsValid?: boolean = true, -): EditableValue { - const [editableValue, setEditableValue] = useState(() => - smartStringify(initialValue), - ); - const [parsedValue, setParsedValue] = useState(initialValue); - const [isValid, setIsValid] = useState(initialIsValid); + externalValue: any, +): [UseEditableValueState, UseEditableValueDispatch] { + const [state, dispatch] = useReducer< + UseEditableValueState, + UseEditableValueAction, + >(useEditableValueReducer, { + editableValue: smartStringify(externalValue), + externalValue, + hasPendingChanges: false, + isValid: true, + parsedValue: externalValue, + }); - const reset = useCallback( - () => { - setEditableValue(smartStringify(initialValue)); - setParsedValue(initialValue); - setIsValid(initialIsValid); - }, - [initialValue, initialIsValid], - ); + if (state.externalValue !== externalValue) { + if (!state.hasPendingChanges) { + dispatch({ + type: 'RESET', + externalValue, + }); + } else { + dispatch({ + type: 'UPDATE', + editableValue: state.editableValue, + externalValue, + }); + } + } - const update = useCallback(newValue => { - let isNewValueValid = false; - let newParsedValue; - try { - newParsedValue = smartParse(newValue); - isNewValueValid = true; - } catch (error) {} - - batchedUpdates(() => { - setEditableValue(sanitizeForParse(newValue)); - if (isNewValueValid) { - setParsedValue(newParsedValue); - } - setIsValid(isNewValueValid); - }); - }, []); - - return { - editableValue, - hasPendingChanges: smartStringify(initialValue) !== editableValue, - isValid, - parsedValue, - reset, - update, - }; + return [state, dispatch]; } export function useIsOverflowing(