From a4212dcdce1b5cbaf8d436871b1fe8968814a358 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 17 Feb 2019 13:07:39 -0800 Subject: [PATCH] Support editable props, state, and context values --- .../dev/app/EditableProps/EditableProps.css | 13 ++ shells/dev/app/EditableProps/EditableProps.js | 57 +++++++++ shells/dev/app/EditableProps/index.js | 5 + shells/dev/app/index.js | 2 + src/backend/agent.js | 46 ++++++- src/backend/renderer.js | 69 +++++++++-- src/backend/types.js | 3 + src/backend/utils.js | 34 ++++++ src/devtools/types.js | 23 ++-- src/devtools/views/Element.js | 5 +- src/devtools/views/InspectedElementTree.css | 28 +++++ src/devtools/views/InspectedElementTree.js | 114 ++++++++++++++++-- src/devtools/views/SelectedElement.js | 63 ++++++++-- src/devtools/views/SettingsContext.js | 1 + src/devtools/views/root.css | 2 + 15 files changed, 428 insertions(+), 37 deletions(-) create mode 100644 shells/dev/app/EditableProps/EditableProps.css create mode 100644 shells/dev/app/EditableProps/EditableProps.js create mode 100644 shells/dev/app/EditableProps/index.js diff --git a/shells/dev/app/EditableProps/EditableProps.css b/shells/dev/app/EditableProps/EditableProps.css new file mode 100644 index 0000000000..b1ebb8ee71 --- /dev/null +++ b/shells/dev/app/EditableProps/EditableProps.css @@ -0,0 +1,13 @@ +.App { + /* GitHub.com frontend fonts */ + font-family: -apple-system, BlinkMacSystemFont, Segoe UI, Helvetica, Arial, + sans-serif, Apple Color Emoji, Segoe UI Emoji, Segoe UI Symbol; + font-size: 14px; + line-height: 1.5; +} + +.Header { + font-size: 1.5rem; + font-weight: bold; + margin-bottom: 0.5rem; +} diff --git a/shells/dev/app/EditableProps/EditableProps.js b/shells/dev/app/EditableProps/EditableProps.js new file mode 100644 index 0000000000..af8d7f901f --- /dev/null +++ b/shells/dev/app/EditableProps/EditableProps.js @@ -0,0 +1,57 @@ +// @flow + +import React, { createContext, Component, Fragment } from 'react'; +import styles from './EditableProps.css'; + +type StatefulFunctionProps = {| count: number |}; + +function StatefulFunction({ count }: StatefulFunctionProps) { + return
  • Count: {count}
    • ; +} + +const BoolContext = createContext(true); +// $FlowFixMe Flow does not yet know about Context.displayName +BoolContext.displayName = 'BoolContext'; + +type Props = {| name: string, toggle: boolean |}; +type State = {| cities: Array, state: string |}; + +class StatefulClass extends Component { + static contextType = BoolContext; + + state: State = { + cities: ['San Francisco', 'San Jose'], + state: 'California', + }; + + handleChange = ({ target }) => + this.setState({ + state: target.value, + }); + + render() { + return ( + +
  • Name: {this.props.name}
    • +
  • Toggle: {this.props.toggle ? 'true' : 'false'}
    • +
  • + State: +
    • +
  • Cities: {this.state.cities.join(', ')}
    • +
  • Context: {this.context ? 'true' : 'false'}
    • +     + ); + } +} + +export default function EditableProps() { + return ( +
    +
    Editable props
    +
      + + +
    +
    + ); +} diff --git a/shells/dev/app/EditableProps/index.js b/shells/dev/app/EditableProps/index.js new file mode 100644 index 0000000000..a57026743b --- /dev/null +++ b/shells/dev/app/EditableProps/index.js @@ -0,0 +1,5 @@ +// @flow + +import EditableProps from './EditableProps'; + +export default EditableProps; diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js index c04798ac80..77a8093b42 100644 --- a/shells/dev/app/index.js +++ b/shells/dev/app/index.js @@ -4,6 +4,7 @@ import { createElement } from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; +import EditableProps from './EditableProps'; import ElementTypes from './ElementTypes'; import InspectableElements from './InspectableElements'; import ToDoList from './ToDoList'; @@ -24,6 +25,7 @@ function mountTestApp() { mountHelper(ToDoList); mountHelper(InspectableElements); mountHelper(ElementTypes); + mountHelper(EditableProps); } function unmountTestApp() { diff --git a/src/backend/agent.js b/src/backend/agent.js index 4876921c78..3848f7b880 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -18,6 +18,18 @@ const debug = (methodName, ...args) => { } }; +type InspectSelectParams = {| + id: number, + rendererID: number, +|}; + +type SetInParams = {| + id: number, + path: Array, + rendererID: number, + value: any, +|}; + export default class Agent extends EventEmitter { _bridge: Bridge = ((null: any): Bridge); _rendererInterfaces: { [key: RendererID]: RendererInterface } = {}; @@ -27,6 +39,9 @@ export default class Agent extends EventEmitter { bridge.addListener('highlightElementInDOM', this.highlightElementInDOM); bridge.addListener('inspectElement', this.inspectElement); + bridge.addListener('overrideContext', this.overrideContext); + bridge.addListener('overrideProps', this.overrideProps); + bridge.addListener('overrideState', this.overrideState); bridge.addListener('selectElement', this.selectElement); bridge.addListener('startInspectingDOM', this.startInspectingDOM); bridge.addListener('stopInspectingDOM', this.stopInspectingDOM); @@ -80,7 +95,7 @@ export default class Agent extends EventEmitter { } }; - inspectElement = ({ id, rendererID }: { id: number, rendererID: number }) => { + inspectElement = ({ id, rendererID }: InspectSelectParams) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); @@ -89,7 +104,7 @@ export default class Agent extends EventEmitter { } }; - selectElement = ({ id, rendererID }: { id: number, rendererID: number }) => { + selectElement = ({ id, rendererID }: InspectSelectParams) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); @@ -98,6 +113,33 @@ export default class Agent extends EventEmitter { } }; + overrideContext = ({ id, path, rendererID, value }: SetInParams) => { + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + } else { + renderer.setInContext(id, path, value); + } + }; + + overrideProps = ({ id, path, rendererID, value }: SetInParams) => { + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + } else { + renderer.setInProps(id, path, value); + } + }; + + overrideState = ({ id, path, rendererID, value }: SetInParams) => { + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + } else { + renderer.setInState(id, path, value); + } + }; + setRendererInterface( rendererID: RendererID, rendererInterface: RendererInterface diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 8114e2a898..f856431317 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -2,7 +2,8 @@ import { gte } from 'semver'; import { - ElementTypeClassOrFunction, + ElementTypeClass, + ElementTypeFunction, ElementTypeContext, ElementTypeForwardRef, ElementTypeMemo, @@ -12,7 +13,7 @@ import { ElementTypeSuspense, } from 'src/devtools/types'; import { getDisplayName, utfEncodeString } from '../utils'; -import { cleanForBridge } from './utils'; +import { cleanForBridge, copyWithSet, setInObject } from './utils'; import { __DEBUG__, TREE_OPERATION_ADD, @@ -193,6 +194,8 @@ export function attach( DEPRECATED_PLACEHOLDER_SYMBOL_STRING, } = ReactSymbols; + const { overrideProps } = renderer; + const debug = (name: string, fiber: Fiber, parentFiber: ?Fiber): void => { if (__DEBUG__) { const fiberData = getDataForFiber(fiber); @@ -295,13 +298,19 @@ export function attach( switch (tag) { case ClassComponent: - case FunctionComponent: case IncompleteClassComponent: + fiberData = { + displayName: getDisplayName(resolvedType), + key, + type: ElementTypeClass, + }; + break; + case FunctionComponent: case IndeterminateComponent: fiberData = { displayName: getDisplayName(resolvedType), key, - type: ElementTypeClassOrFunction, + type: ElementTypeFunction, }; break; case ForwardRef: @@ -1072,7 +1081,7 @@ export function attach( tag === IncompleteClassComponent || tag === IndeterminateComponent ) { - if (stateNode && Object.keys(stateNode.context).length > 0) { + if (stateNode && stateNode.context != null) { context = stateNode.context; } } else if ( @@ -1135,7 +1144,7 @@ export function attach( id, // Does the current renderer support editable props/state/hooks? - canEditValues: false, // TODO + canEditFunctionProps: typeof overrideProps === 'function', // Inspectable properties. // TODO Review sanitization approach for the below inspectable values. @@ -1156,6 +1165,49 @@ export function attach( }; } + function setInProps(id: number, path: Array, value: any) { + const fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); + if (fiber !== null) { + const instance = fiber.stateNode; + if (instance === null) { + if (typeof overrideProps === 'function') { + overrideProps(fiber, path, value); + } + } else { + fiber.pendingProps = copyWithSet(instance.props, path, value); + instance.forceUpdate(); + } + } + } + + function setInState(id: number, path: Array, value: any) { + const fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); + if (fiber !== null) { + const instance = fiber.stateNode; + setInObject(instance.state, path, value); + instance.forceUpdate(); + } + } + + function setInContext(id: number, path: Array, value: any) { + // To simplify hydration and display of primative context values (e.g. number, string) + // the inspectElement() method wraps context in a {value: ...} object. + // We need to remove the first part of the path (the "value") before continuing. + path = path.slice(1); + + const fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); + if (fiber !== null) { + const instance = fiber.stateNode; + if (path.length === 0) { + // Simple context value + instance.context = value; + } else { + setInObject(instance.context, path, value); + } + instance.forceUpdate(); + } + } + return { getFiberIDFromNative, getNativeFromReactElement, @@ -1164,7 +1216,10 @@ export function attach( inspectElement, selectElement, cleanup, - walkTree, renderer, + setInContext, + setInProps, + setInState, + walkTree, }; } diff --git a/src/backend/types.js b/src/backend/types.js index f9dbbe0e49..7578554e4e 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -55,6 +55,9 @@ export type RendererInterface = { inspectElement: (id: number) => InspectedElement | null, renderer: ReactRenderer | null, selectElement: (id: number) => void, + setInProps: (id: number, path: Array, value: any) => void, + setInState: (id: number, path: Array, value: any) => void, + setInContext: (id: number, path: Array, value: any) => void, walkTree: () => void, }; diff --git a/src/backend/utils.js b/src/backend/utils.js index dea4abfd6e..ddb11eca12 100644 --- a/src/backend/utils.js +++ b/src/backend/utils.js @@ -16,3 +16,37 @@ export function cleanForBridge(data: Object | null): DehydratedData | null { return null; } } + +export function copyWithSet( + obj: Object | Array, + path: Array, + value: any, + index: number = 0 +): Object | Array { + if (index >= path.length) { + return value; + } + const key = path[index]; + const updated = Array.isArray(obj) ? obj.slice() : { ...obj }; + // $FlowFixMe number or string is fine here + updated[key] = copyWithSet(obj[key], path, value, index + 1); + return updated; +} + +export function setInObject( + object: Object, + path: Array, + value: any +) { + const last = path.pop(); + if (object != null) { + const parent: Object = path.reduce( + // $FlowFixMe + (reduced, attribute) => reduced[attribute], + object + ); + if (parent) { + parent[last] = value; + } + } +} diff --git a/src/devtools/types.js b/src/devtools/types.js index 59cddbfd10..9b08e5f30e 100644 --- a/src/devtools/types.js +++ b/src/devtools/types.js @@ -1,18 +1,19 @@ // @flow -export const ElementTypeClassOrFunction = 1; -export const ElementTypeContext = 2; -export const ElementTypeForwardRef = 3; -export const ElementTypeMemo = 4; -export const ElementTypeOtherOrUnknown = 5; -export const ElementTypeProfiler = 6; -export const ElementTypeRoot = 7; -export const ElementTypeSuspense = 8; +export const ElementTypeClass = 1; +export const ElementTypeFunction = 2; +export const ElementTypeContext = 3; +export const ElementTypeForwardRef = 4; +export const ElementTypeMemo = 5; +export const ElementTypeOtherOrUnknown = 6; +export const ElementTypeProfiler = 7; +export const ElementTypeRoot = 8; +export const ElementTypeSuspense = 9; // Different types of elements displayed in the Elements tree. // These types may be used to visually distinguish types, // or to enable/disable certain functionality. -export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8; +export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9; // Each element on the frontend corresponds to a Fiber on the backend. // Some of its information (e.g. id, type, displayName) come from the backend. @@ -47,8 +48,8 @@ export type Owner = {| export type InspectedElement = {| id: number, - // Does the current renderer support editable props/state/hooks? - canEditValues: boolean, + // Does the current renderer support editable function props? + canEditFunctionProps: boolean, // Inspectable properties. context: Object | null, diff --git a/src/devtools/views/Element.js b/src/devtools/views/Element.js index 03fc88251a..7006032af1 100644 --- a/src/devtools/views/Element.js +++ b/src/devtools/views/Element.js @@ -1,7 +1,7 @@ // @flow import React, { Fragment, useCallback, useContext, useMemo } from 'react'; -import { ElementTypeClassOrFunction } from 'src/devtools/types'; +import { ElementTypeClass, ElementTypeFunction } from 'src/devtools/types'; import { createRegExp } from './utils'; import { TreeContext } from './TreeContext'; @@ -42,7 +42,8 @@ export default function ElementView({ index, style }: Props) { ); const isSelected = selectedElementID === id; - const showDollarR = isSelected && type === ElementTypeClassOrFunction; + const showDollarR = + isSelected && (type === ElementTypeClass || type === ElementTypeFunction); // TODO styles.SelectedElement is 100% width but it doesn't take horizontal overflow into account. diff --git a/src/devtools/views/InspectedElementTree.css b/src/devtools/views/InspectedElementTree.css index 0008e1ada2..014fdfc2b0 100644 --- a/src/devtools/views/InspectedElementTree.css +++ b/src/devtools/views/InspectedElementTree.css @@ -7,16 +7,44 @@ } .Item { + display: flex; } .Name { color: var(--color-attribute-name); + flex: 0 0 auto; +} +.Name:after { + content: ': '; + color: var(--color-text-color); + margin-right: 0.5rem; } .Value { color: var(--color-attribute-value); } +.ValueInputLabel { + flex: 1 1 100%; +} +.ValueInputLabel:focus-within { + background-color: var(--color-button-background-focus); +} + +.ValueInput { + background: none; + border: 1px solid transparent; + color: var(--color-attribute-editable-value); + border-radius: 0.125rem; + width: 100%; + font-family: var(--font-family-monospace); + font-size: var(--font-size-monospace-normal); +} +.ValueInput:focus { + background-color: var(--color-button-background-focus); + outline: none; +} + .None { color: var(--color-dimmer); font-style: italic; diff --git a/src/devtools/views/InspectedElementTree.js b/src/devtools/views/InspectedElementTree.js index b79fa52f18..a6b2f8648f 100644 --- a/src/devtools/views/InspectedElementTree.js +++ b/src/devtools/views/InspectedElementTree.js @@ -1,19 +1,23 @@ // @flow -import React from 'react'; +import React, { useCallback, useState } from 'react'; import { getMetaValueLabel } from './utils'; import { meta } from '../../hydration'; import styles from './InspectedElementTree.css'; +type OverrideValueFn = (path: Array, value: any) => void; + type Props = {| data: Object | null, label: string, + overrideValueFn?: ?OverrideValueFn, showWhenEmpty?: boolean, |}; export default function InspectedElementTree({ data, label, + overrideValueFn, showWhenEmpty = false, }: Props) { const isEmpty = data === null || Object.keys(data).length === 0; @@ -33,6 +37,8 @@ export default function InspectedElementTree({ key={name} depth={1} name={name} + overrideValueFn={overrideValueFn} + path={[name]} value={(data: any)[name]} /> ))} @@ -44,10 +50,18 @@ export default function InspectedElementTree({ type KeyValueProps = {| depth: number, name: string, + overrideValueFn?: ?OverrideValueFn, + path?: Array, value: any, |}; -export function KeyValue({ depth, name, value }: KeyValueProps) { +export function KeyValue({ + depth, + name, + overrideValueFn, + path = [], + value, +}: KeyValueProps) { const dataType = typeof value; const isSimpleType = dataType === 'number' || @@ -72,14 +86,24 @@ export function KeyValue({ depth, name, value }: KeyValueProps) { children = (
    - {name}:{' '} - {displayValue} + {name} + {typeof overrideValueFn === 'function' ? ( + + ) : ( + {displayValue} + )}
    ); } else if (value.hasOwnProperty(meta.type)) { + // TODO Is this type even necessary? Can we just drop it? children = (
    - {name}:{' '} + {name} {getMetaValueLabel(value)}
    ); @@ -90,6 +114,8 @@ export function KeyValue({ depth, name, value }: KeyValueProps) { key={index} depth={depth + 1} name={index} + overrideValueFn={overrideValueFn} + path={path.concat(index)} value={value[index]} /> )); @@ -99,13 +125,21 @@ export function KeyValue({ depth, name, value }: KeyValueProps) { className={styles.Item} style={{ paddingLeft }} > - {name}: Array + {name} + Array ); } else { // $FlowFixMe children = Object.entries(value).map(([name, value]) => ( - + )); children.unshift(
    - {name}: Object + {name} + Object
    ); } @@ -121,3 +156,66 @@ export function KeyValue({ depth, name, value }: KeyValueProps) { return children; } + +type EditableValueProps = {| + dataType: string, + overrideValueFn: OverrideValueFn, + path: Array, + value: any, +|}; + +function EditableValue({ + dataType, + overrideValueFn, + path, + value, +}: EditableValueProps) { + const [editableValue, setEditableValue] = useState(value); + + const handleChange = useCallback( + ({ target }) => { + if (dataType === 'boolean') { + setEditableValue(target.checked); + overrideValueFn(path, target.checked); + } else if (dataType === 'number') { + setEditableValue(parseFloat(target.value)); + } else { + setEditableValue(target.value); + } + }, + [dataType, setEditableValue] + ); + + const handleKeyPress = useCallback( + ({ key }) => { + if (key === 'Enter') { + overrideValueFn(path, editableValue); + } + }, + [path, editableValue, overrideValueFn] + ); + + const handleKeyDown = useCallback(event => event.stopPropagation(), []); + + // Render different input types based on the dataType + let type = 'text'; + if (dataType === 'boolean') { + type = 'checkbox'; + } else if (dataType === 'number') { + type = 'number'; + } + + return ( + + ); +} diff --git a/src/devtools/views/SelectedElement.js b/src/devtools/views/SelectedElement.js index 27cd085e70..ab57d57e94 100644 --- a/src/devtools/views/SelectedElement.js +++ b/src/devtools/views/SelectedElement.js @@ -15,9 +15,10 @@ import HooksTree from './HooksTree'; import InspectedElementTree from './InspectedElementTree'; import { hydrate } from 'src/hydration'; import styles from './SelectedElement.css'; +import { ElementTypeClass, ElementTypeFunction } from '../types'; import type { InspectedElement } from '../types'; -import type { DehydratedData } from 'src/devtools/types'; +import type { DehydratedData, Element } from 'src/devtools/types'; export type Props = {||}; @@ -87,26 +88,74 @@ export default function SelectedElement(_: Props) { )} {inspectedElement !== null && ( - + )} ); } type InspectedElementViewProps = {| + element: Element, inspectedElement: InspectedElement, |}; -function InspectedElementView({ inspectedElement }: InspectedElementViewProps) { - let { context, hooks, owners, props, state } = inspectedElement; +function InspectedElementView({ + element, + inspectedElement, +}: InspectedElementViewProps) { + const { id, type } = element; + const { context, hooks, owners, props, state } = inspectedElement; + const { ownerStack } = useContext(TreeContext); + const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); + + let overrideContextFn = null; + let overridePropsFn = null; + let overrideStateFn = null; + if (type === ElementTypeClass) { + overrideContextFn = (path: Array, value: any) => { + const rendererID = store.getRendererIDForElement(id); + bridge.send('overrideContext', { id, path, rendererID, value }); + }; + overridePropsFn = (path: Array, value: any) => { + const rendererID = store.getRendererIDForElement(id); + bridge.send('overrideProps', { id, path, rendererID, value }); + }; + overrideStateFn = (path: Array, value: any) => { + const rendererID = store.getRendererIDForElement(id); + bridge.send('overrideState', { id, path, rendererID, value }); + }; + } else if (type === ElementTypeFunction) { + // TODO Only enable this if renderer.canEditFunctionProps is true! + overridePropsFn = (path: Array, value: any) => { + const rendererID = store.getRendererIDForElement(id); + bridge.send('overrideProps', { id, path, rendererID, value }); + }; + } return (
    - - + + - + {ownerStack.length === 0 && owners !== null && owners.length > 0 && (
    diff --git a/src/devtools/views/SettingsContext.js b/src/devtools/views/SettingsContext.js index 2da5bc5822..17f25f5a47 100644 --- a/src/devtools/views/SettingsContext.js +++ b/src/devtools/views/SettingsContext.js @@ -118,6 +118,7 @@ function updateDisplayDensity(displayDensity: DisplayDensity): void { function updateThemeVariables(theme: Theme): void { updateStyleHelper(theme, 'color-attribute-name'); updateStyleHelper(theme, 'color-attribute-value'); + updateStyleHelper(theme, 'color-attribute-editable-value'); updateStyleHelper(theme, 'color-background'); updateStyleHelper(theme, 'color-border'); updateStyleHelper(theme, 'color-button-background'); diff --git a/src/devtools/views/root.css b/src/devtools/views/root.css index f8ec068351..cf5e4d939a 100644 --- a/src/devtools/views/root.css +++ b/src/devtools/views/root.css @@ -6,6 +6,7 @@ /* Light theme */ --light-color-attribute-name: #ef6632; --light-color-attribute-value: #1a1aa6; + --light-color-attribute-editable-value: #1a1aa6; --light-color-background: #ffffff; --light-color-button-background: #ffffff; --light-color-button-background-focus: #ebf1fb; @@ -31,6 +32,7 @@ /* Dark theme */ --dark-color-attribute-name: #9d87d2; --dark-color-attribute-value: #cedae0; + --dark-color-attribute-editable-value: yellow; --dark-color-background: #282c34; --dark-color-button-background: #282c34; --dark-color-button-background-focus: #3d424a;