Skip to content

Commit

Permalink
refactor[devtools]: copy to clipboard only on frontend side (facebook…
Browse files Browse the repository at this point in the history
…#26604)

Fixes facebook#26500

## Summary
- No more using `clipboard-js` from the backend side, now emitting
custom `saveToClipboard` event, also adding corresponding listener in
`store.js`
- Not migrating to `navigator.clipboard` api yet, there were some issues
with using it on Chrome, will add more details to
facebook#26539

## How did you test this change?
- Tested on Chrome, Firefox, Edge
- Tested on standalone electron app: seems like context menu is not
expected to work there (cannot right-click on value, the menu is not
appearing), other logic (pressing on copy icon) was not changed
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent 6f9ba65 commit 9525a7b
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,3 @@ if (IS_FIREFOX) {
}
}
}

if (typeof exportFunction === 'function') {
// eslint-disable-next-line no-undef
exportFunction(
text => {
// Call clipboard.writeText from the extension content script
// (as it has the clipboardWrite permission) and return a Promise
// accessible to the webpage js code.
return new window.Promise((resolve, reject) =>
window.navigator.clipboard.writeText(text).then(resolve, reject),
);
},
window.wrappedJSObject.__REACT_DEVTOOLS_GLOBAL_HOOK__,
{defineAs: 'clipboardCopyText'},
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1801,7 +1801,7 @@ describe('InspectedElement', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify(nestedObject),
JSON.stringify(nestedObject, undefined, 2),
);

global.mockClipboardCopy.mockReset();
Expand All @@ -1811,7 +1811,7 @@ describe('InspectedElement', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify(nestedObject.a.b),
JSON.stringify(nestedObject.a.b, undefined, 2),
);
});

Expand Down Expand Up @@ -1894,7 +1894,7 @@ describe('InspectedElement', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify('123n'),
JSON.stringify('123n', undefined, 2),
);

global.mockClipboardCopy.mockReset();
Expand All @@ -1904,7 +1904,7 @@ describe('InspectedElement', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify({0: 100, 1: -100, 2: 0}),
JSON.stringify({0: 100, 1: -100, 2: 0}, undefined, 2),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('InspectedElementContext', () => {

async function read(
id: number,
path?: Array<string | number> = null,
path: Array<string | number> = null,
): Promise<Object> {
const rendererID = ((store.getRendererIDForElement(id): any): number);
const promise = backendAPI
Expand Down Expand Up @@ -826,7 +826,7 @@ describe('InspectedElementContext', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify(nestedObject),
JSON.stringify(nestedObject, undefined, 2),
);

global.mockClipboardCopy.mockReset();
Expand All @@ -842,7 +842,7 @@ describe('InspectedElementContext', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify(nestedObject.a.b),
JSON.stringify(nestedObject.a.b, undefined, 2),
);
});

Expand Down Expand Up @@ -932,7 +932,7 @@ describe('InspectedElementContext', () => {
jest.runOnlyPendingTimers();
expect(global.mockClipboardCopy).toHaveBeenCalledTimes(1);
expect(global.mockClipboardCopy).toHaveBeenCalledWith(
JSON.stringify({0: 100, 1: -100, 2: 0}),
JSON.stringify({0: 100, 1: -100, 2: 0}, undefined, 2),
);
});
});
8 changes: 7 additions & 1 deletion packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,13 @@ export default class Agent extends EventEmitter<{
if (renderer == null) {
console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
} else {
renderer.copyElementPath(id, path);
const value = renderer.getSerializedElementValueByPath(id, path);

if (value != null) {
this._bridge.send('saveToClipboard', value);
} else {
console.warn(`Unable to obtain serialized value for element "${id}"`);
}
}
};

Expand Down
13 changes: 9 additions & 4 deletions packages/react-devtools-shared/src/backend/legacy/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import {
import {getUID, utfEncodeString, printOperationsArray} from '../../utils';
import {
cleanForBridge,
copyToClipboard,
copyWithDelete,
copyWithRename,
copyWithSet,
serializeToString,
} from '../utils';
import {
deletePathInObject,
Expand Down Expand Up @@ -701,10 +701,15 @@ export function attach(
}
}

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

return serializeToString(valueToCopy);
}
}

Expand Down Expand Up @@ -1105,7 +1110,7 @@ export function attach(
clearErrorsForFiberID,
clearWarningsForFiberID,
cleanup,
copyElementPath,
getSerializedElementValueByPath,
deletePath,
flushInitialOperations,
getBestMatchForTrackedPath,
Expand Down
26 changes: 16 additions & 10 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@ import {
utfEncodeString,
} from 'react-devtools-shared/src/utils';
import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
import {gt, gte} from 'react-devtools-shared/src/backend/utils';
import {
gt,
gte,
serializeToString,
} from 'react-devtools-shared/src/backend/utils';
import {
cleanForBridge,
copyToClipboard,
copyWithDelete,
copyWithRename,
copyWithSet,
Expand Down Expand Up @@ -809,7 +812,7 @@ export function attach(
name: string,
fiber: Fiber,
parentFiber: ?Fiber,
extraString?: string = '',
extraString: string = '',
): void => {
if (__DEBUG__) {
const displayName =
Expand Down Expand Up @@ -3544,14 +3547,17 @@ export function attach(
}
}

function copyElementPath(id: number, path: Array<string | number>): void {
function getSerializedElementValueByPath(
id: number,
path: Array<string | number>,
): ?string {
if (isMostRecentlyInspectedElement(id)) {
copyToClipboard(
getInObject(
((mostRecentlyInspectedElement: any): InspectedElement),
path,
),
const valueToCopy = getInObject(
((mostRecentlyInspectedElement: any): InspectedElement),
path,
);

return serializeToString(valueToCopy);
}
}

Expand Down Expand Up @@ -4494,7 +4500,7 @@ export function attach(
clearErrorsAndWarnings,
clearErrorsForFiberID,
clearWarningsForFiberID,
copyElementPath,
getSerializedElementValueByPath,
deletePath,
findNativeNodesForFiberID,
flushInitialOperations,
Expand Down
5 changes: 4 additions & 1 deletion packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,6 @@ export type RendererInterface = {
clearErrorsAndWarnings: () => void,
clearErrorsForFiberID: (id: number) => void,
clearWarningsForFiberID: (id: number) => void,
copyElementPath: (id: number, path: Array<string | number>) => void,
deletePath: (
type: Type,
id: number,
Expand All @@ -367,6 +366,10 @@ export type RendererInterface = {
getProfilingData(): ProfilingDataBackend,
getOwnersList: (id: number) => Array<SerializedElement> | null,
getPathForElement: (id: number) => Array<PathFrame> | null,
getSerializedElementValueByPath: (
id: number,
path: Array<string | number>,
) => ?string,
handleCommitFiberRoot: (fiber: Object, commitPriority?: number) => void,
handleCommitFiberUnmount: (fiber: Object) => void,
handlePostCommitFiberRoot: (fiber: Object) => void,
Expand Down
50 changes: 20 additions & 30 deletions packages/react-devtools-shared/src/backend/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
* @flow
*/

import {copy} from 'clipboard-js';
import {compareVersions} from 'compare-versions';
import {dehydrate} from '../hydration';
import isArray from 'shared/isArray';
Expand All @@ -18,7 +17,7 @@ import type {DehydratedData} from 'react-devtools-shared/src/devtools/views/Comp
export function cleanForBridge(
data: Object | null,
isPathAllowed: (path: Array<string | number>) => boolean,
path?: Array<string | number> = [],
path: Array<string | number> = [],
): DehydratedData | null {
if (data !== null) {
const cleanedPaths: Array<Array<string | number>> = [];
Expand All @@ -41,23 +40,6 @@ export function cleanForBridge(
}
}

export function copyToClipboard(value: any): void {
const safeToCopy = serializeToString(value);
const text = safeToCopy === undefined ? 'undefined' : safeToCopy;
const {clipboardCopyText} = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;

// On Firefox navigator.clipboard.writeText has to be called from
// the content script js code (because it requires the clipboardWrite
// permission to be allowed out of a "user handling" callback),
// clipboardCopyText is an helper injected into the page from.
// injectGlobalHook.
if (typeof clipboardCopyText === 'function') {
clipboardCopyText(text).catch(err => {});
} else {
copy(text);
}
}

export function copyWithDelete(
obj: Object | Array<any>,
path: Array<string | number>,
Expand Down Expand Up @@ -144,20 +126,28 @@ export function getEffectDurations(root: Object): {
}

export function serializeToString(data: any): string {
if (data === undefined) {
return 'undefined';
}

const cache = new Set<mixed>();
// Use a custom replacer function to protect against circular references.
return JSON.stringify(data, (key, value) => {
if (typeof value === 'object' && value !== null) {
if (cache.has(value)) {
return;
return JSON.stringify(
data,
(key, value) => {
if (typeof value === 'object' && value !== null) {
if (cache.has(value)) {
return;
}
cache.add(value);
}
cache.add(value);
}
if (typeof value === 'bigint') {
return value.toString() + 'n';
}
return value;
});
if (typeof value === 'bigint') {
return value.toString() + 'n';
}
return value;
},
2,
);
}

// Formats an array of args with a style for console methods, using
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ export type BackendEvents = {
profilingData: [ProfilingDataBackend],
profilingStatus: [boolean],
reloadAppForProfiling: [],
saveToClipboard: [string],
selectFiber: [number],
shutdown: [],
stopInspectingNative: [boolean],
Expand Down
8 changes: 8 additions & 0 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* @flow
*/

import {copy} from 'clipboard-js';
import EventEmitter from '../events';
import {inspect} from 'util';
import {
Expand Down Expand Up @@ -272,6 +273,8 @@ export default class Store extends EventEmitter<{

bridge.addListener('backendVersion', this.onBridgeBackendVersion);
bridge.send('getBackendVersion');

bridge.addListener('saveToClipboard', this.onSaveToClipboard);
}

// This is only used in tests to avoid memory leaks.
Expand Down Expand Up @@ -1362,6 +1365,7 @@ export default class Store extends EventEmitter<{
);
bridge.removeListener('backendVersion', this.onBridgeBackendVersion);
bridge.removeListener('bridgeProtocol', this.onBridgeProtocol);
bridge.removeListener('saveToClipboard', this.onSaveToClipboard);

if (this._onBridgeProtocolTimeoutID !== null) {
clearTimeout(this._onBridgeProtocolTimeoutID);
Expand Down Expand Up @@ -1422,6 +1426,10 @@ export default class Store extends EventEmitter<{
this.emit('unsupportedBridgeProtocolDetected');
};

onSaveToClipboard: (text: string) => void = text => {
copy(text);
};

// The Store should never throw an Error without also emitting an event.
// Otherwise Store errors will be invisible to users,
// but the downstream errors they cause will be reported as bugs.
Expand Down

0 comments on commit 9525a7b

Please sign in to comment.