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 context menu #17608

Merged
merged 10 commits into from
Dec 18, 2019
Prev Previous commit
Next Next commit
Added backend support for copying a value at a specific path for the …
…inspected element
  • Loading branch information
Brian Vaughn committed Dec 17, 2019
commit 4766503de495f68dc7df0e7f2861a576a3dac135
16 changes: 16 additions & 0 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ type ElementAndRendererID = {|
rendererID: number,
|};

type CopyElementParams = {|
id: number,
path: Array<string | number>,
rendererID: number,
|};

type InspectElementParams = {|
id: number,
path?: Array<string | number>,
Expand Down Expand Up @@ -126,6 +132,7 @@ export default class Agent extends EventEmitter<{|

this._bridge = bridge;

bridge.addListener('copyElementPath', this.copyElementPath);
bridge.addListener('getProfilingData', this.getProfilingData);
bridge.addListener('getProfilingStatus', this.getProfilingStatus);
bridge.addListener('getOwnersList', this.getOwnersList);
Expand Down Expand Up @@ -173,6 +180,15 @@ export default class Agent extends EventEmitter<{|
return this._rendererInterfaces;
}

copyElementPath = ({id, path, rendererID}: CopyElementParams) => {
const renderer = this._rendererInterfaces[rendererID];
if (renderer == null) {
console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
} else {
renderer.copyElementPath(id, path);
}
};

getInstanceAndStyle({
id,
rendererID,
Expand Down
15 changes: 13 additions & 2 deletions packages/react-devtools-shared/src/backend/legacy/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* @flow
*/

import {copy} from 'clipboard-js';
import {
ElementTypeClass,
ElementTypeFunction,
Expand All @@ -15,8 +16,8 @@ import {
ElementTypeOtherOrUnknown,
} from 'react-devtools-shared/src/types';
import {getUID, utfEncodeString, printOperationsArray} from '../../utils';
import {cleanForBridge, copyWithSet} from '../utils';
import {getDisplayName} from 'react-devtools-shared/src/utils';
import {cleanForBridge, copyWithSet, safeSerialize} from '../utils';
import {getDisplayName, getInObject} from 'react-devtools-shared/src/utils';
import {
__DEBUG__,
TREE_OPERATION_ADD,
Expand Down Expand Up @@ -649,6 +650,15 @@ export function attach(
}
}

function copyElementPath(id: number, path: Array<string | number>): void {
const inspectedElement = inspectElementRaw(id);
if (inspectedElement !== null) {
const value = getInObject(inspectedElement, path);

copy(safeSerialize(value));
}
}

function inspectElement(
id: number,
path?: Array<string | number>,
Expand Down Expand Up @@ -927,6 +937,7 @@ export function attach(

return {
cleanup,
copyElementPath,
flushInitialOperations,
getBestMatchForTrackedPath,
getFiberIDForNative: getInternalIDForNative,
Expand Down
17 changes: 16 additions & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import {gte} from 'semver';
import {copy} from 'clipboard-js';
import {
ComponentFilterDisplayName,
ComponentFilterElementType,
Expand All @@ -34,7 +35,7 @@ import {
utfEncodeString,
} from 'react-devtools-shared/src/utils';
import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
import {cleanForBridge, copyWithSet} from './utils';
import {cleanForBridge, copyWithSet, safeSerialize} from './utils';
import {
__DEBUG__,
SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
Expand Down Expand Up @@ -2488,6 +2489,19 @@ export function attach(
}
}

function copyElementPath(id: number, path: Array<string | number>): void {
const isCurrent = isMostRecentlyInspectedElementCurrent(id);

if (isCurrent) {
const value = getInObject(
((mostRecentlyInspectedElement: any): InspectedElement),
path,
);

copy(safeSerialize(value));
}
}

function inspectElement(
id: number,
path?: Array<string | number>,
Expand Down Expand Up @@ -3129,6 +3143,7 @@ export function attach(

return {
cleanup,
copyElementPath,
findNativeNodesForFiberID,
flushInitialOperations,
getBestMatchForTrackedPath,
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ export type InstanceAndStyle = {|

export type RendererInterface = {
cleanup: () => void,
copyElementPath: (id: number, path: Array<string | number>) => void,
findNativeNodesForFiberID: FindNativeNodesForFiberID,
flushInitialOperations: () => void,
getBestMatchForTrackedPath: () => PathMatch | null,
Expand Down
13 changes: 13 additions & 0 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,16 @@ export function copyWithSet(
updated[key] = copyWithSet(obj[key], path, value, index + 1);
return updated;
}

export function safeSerialize(data: any): string {
const cache = new Set();
return JSON.stringify(data, (key, value) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain what makes this safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe "safe" is a poor choice of words. This function protects against circular references, which would otherwise cause JSON.stringify to throw.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay, I was wondering in cases of using Symbols or functions etc, would it be safe for those too? Naming things is hard :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be "safe" in that case, it would just not copy anything meaningful to the clipboard. (If a value can't be serialized, JSON.stringify would just return undefined.)

I renamed "safeSerialize" to "serializeToString" though and added an inline comment about its purpose.

Choose a reason for hiding this comment

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

That’s a cool trick @bvaughn 👌🏽

if (typeof value === 'object' && value !== null) {
if (cache.has(value)) {
return;
}
cache.add(value);
}
return value;
});
}
6 changes: 6 additions & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ type OverrideSuspense = {|
forceFallback: boolean,
|};

type CopyElementPathParams = {|
...ElementAndRendererID,
path: Array<string | number>,
|};

type InspectElementParams = {|
...ElementAndRendererID,
path?: Array<string | number>,
Expand Down Expand Up @@ -95,6 +100,7 @@ type BackendEvents = {|

type FrontendEvents = {|
clearNativeElementHighlight: [],
copyElementPath: [CopyElementPathParams],
getOwnersList: [ElementAndRendererID],
getProfilingData: [{|rendererID: RendererID|}],
getProfilingStatus: [],
Expand Down