From 1d2856c0aa57390cb5e8dae47a800c65156215fd Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 20 Aug 2024 21:57:16 -0400 Subject: [PATCH 1/3] =?UTF-8?q?Remove=20=C6=92=20prefix=20when=20displayin?= =?UTF-8?q?g=20functions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a cute short cut and there's precedence in how Chrome prints functions in the console *if* the function's toString would've had a function prefix like if it was a function declaration or expression. It does not do this for arrow functions or object methods. Elsewhere in the JS ecosystem this isn't really used anywhere. It invites more questions than it answers. The parenthesis and curlies are enough. There's no ambiguity here since strings have quotations. It looks better with just its object method form. Keeping it simple seems best. --- .../src/__tests__/inspectedElement-test.js | 28 +++++++++---------- .../__tests__/legacy/inspectElement-test.js | 12 ++++---- packages/react-devtools-shared/src/utils.js | 4 +-- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 843c6dff9c08f..04a4740af708f 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -697,8 +697,8 @@ describe('InspectedElement', () => { expect(inspectedElement.props).toMatchInlineSnapshot(` { "anonymous_fn": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, "array_buffer": Dehydrated { "preview_short": ArrayBuffer(3), @@ -715,8 +715,8 @@ describe('InspectedElement', () => { "preview_long": 123n, }, "bound_fn": Dehydrated { - "preview_short": ƒ bound exampleFunction() {}, - "preview_long": ƒ bound exampleFunction() {}, + "preview_short": bound exampleFunction() {}, + "preview_long": bound exampleFunction() {}, }, "data_view": Dehydrated { "preview_short": DataView(3), @@ -727,8 +727,8 @@ describe('InspectedElement', () => { "preview_long": Tue Dec 31 2019 23:42:42 GMT+0000 (Coordinated Universal Time), }, "fn": Dehydrated { - "preview_short": ƒ exampleFunction() {}, - "preview_long": ƒ exampleFunction() {}, + "preview_short": exampleFunction() {}, + "preview_long": exampleFunction() {}, }, "html_element": Dehydrated { "preview_short":
, @@ -778,8 +778,8 @@ describe('InspectedElement', () => { "Symbol(name)": "hello", }, "proxy": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, "react_element": Dehydrated { "preview_short": , @@ -2018,16 +2018,16 @@ describe('InspectedElement', () => { { "proxy": { "$$typeof": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, "Symbol(Symbol.iterator)": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, "constructor": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, }, } diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index 243759e5b02e7..a573e0b6bd2c5 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -212,8 +212,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement.props).toMatchInlineSnapshot(` { "anonymous_fn": Dehydrated { - "preview_short": ƒ () {}, - "preview_long": ƒ () {}, + "preview_short": () {}, + "preview_long": () {}, }, "array_buffer": Dehydrated { "preview_short": ArrayBuffer(3), @@ -230,8 +230,8 @@ describe('InspectedElementContext', () => { "preview_long": 123n, }, "bound_fn": Dehydrated { - "preview_short": ƒ bound exampleFunction() {}, - "preview_long": ƒ bound exampleFunction() {}, + "preview_short": bound exampleFunction() {}, + "preview_long": bound exampleFunction() {}, }, "data_view": Dehydrated { "preview_short": DataView(3), @@ -242,8 +242,8 @@ describe('InspectedElementContext', () => { "preview_long": Thu Jan 01 1970 00:00:00 GMT+0000 (Coordinated Universal Time), }, "fn": Dehydrated { - "preview_short": ƒ exampleFunction() {}, - "preview_long": ƒ exampleFunction() {}, + "preview_short": exampleFunction() {}, + "preview_long": exampleFunction() {}, }, "html_element": Dehydrated { "preview_short":
, diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 8e6f52f6c0f8a..0caaaac41371e 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -852,9 +852,7 @@ export function formatDataForPreview( case 'html_element': return `<${truncateForDisplay(data.tagName.toLowerCase())} />`; case 'function': - return truncateForDisplay( - `ƒ ${typeof data.name === 'function' ? '' : data.name}() {}`, - ); + return `${truncateForDisplay(typeof data.name === 'function' ? '' : data.name)}() {}`; case 'string': return `"${data}"`; case 'bigint': From 2d493e9a3cfbc287161cd1f14eb0cc5f16a5da11 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 20 Aug 2024 22:28:12 -0400 Subject: [PATCH 2/3] Use arrow function notation when a name is missing While this might not be an arrow function it's a common enough annotation for an unnamed function that people would get it. --- .../src/__tests__/inspectedElement-test.js | 20 +++++++++---------- .../__tests__/legacy/inspectElement-test.js | 4 ++-- packages/react-devtools-shared/src/utils.js | 5 ++++- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 04a4740af708f..cd777f8333763 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -697,8 +697,8 @@ describe('InspectedElement', () => { expect(inspectedElement.props).toMatchInlineSnapshot(` { "anonymous_fn": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, "array_buffer": Dehydrated { "preview_short": ArrayBuffer(3), @@ -778,8 +778,8 @@ describe('InspectedElement', () => { "Symbol(name)": "hello", }, "proxy": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, "react_element": Dehydrated { "preview_short": , @@ -2018,16 +2018,16 @@ describe('InspectedElement', () => { { "proxy": { "$$typeof": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, "Symbol(Symbol.iterator)": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, "constructor": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, }, } diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index a573e0b6bd2c5..d97c1897892f1 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -212,8 +212,8 @@ describe('InspectedElementContext', () => { expect(inspectedElement.props).toMatchInlineSnapshot(` { "anonymous_fn": Dehydrated { - "preview_short": () {}, - "preview_long": () {}, + "preview_short": () => {}, + "preview_long": () => {}, }, "array_buffer": Dehydrated { "preview_short": ArrayBuffer(3), diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 0caaaac41371e..876e185bb505d 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -852,7 +852,10 @@ export function formatDataForPreview( case 'html_element': return `<${truncateForDisplay(data.tagName.toLowerCase())} />`; case 'function': - return `${truncateForDisplay(typeof data.name === 'function' ? '' : data.name)}() {}`; + if (typeof data.name === 'function' || data.name === '') { + return '() => {}'; + } + return `${truncateForDisplay(data.name)}() {}`; case 'string': return `"${data}"`; case 'bigint': From 7b58f2a8ddb3302bb60f6f4e92465d348b0de264 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 20 Aug 2024 23:50:32 -0400 Subject: [PATCH 3/3] Make functions clickable and style them like a link Hover state makes it a little clearer that it's clickable and follows the pattern of the owner list. --- .../devtools/views/Components/KeyValue.css | 8 ++++ .../src/devtools/views/Components/KeyValue.js | 42 +++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css index d6851be672cff..0c0f5e852d9ee 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.css @@ -34,8 +34,16 @@ overflow: hidden; text-overflow: ellipsis; flex: 1; + cursor: pointer; + border-radius: 0.125rem; + padding: 0px 2px; } +.Link:hover { + background-color: var(--color-background-hover); +} + + .ExpandCollapseToggleSpacer { flex: 0 0 1rem; width: 1rem; 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 16e619cfb0847..af9e4b27e6181 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js @@ -27,7 +27,9 @@ import isArray from 'react-devtools-shared/src/isArray'; import {InspectedElementContext} from './InspectedElementContext'; import {PROTOCOLS_SUPPORTED_AS_LINKS_IN_KEY_VALUE} from './constants'; import KeyValueContextMenuContainer from './KeyValueContextMenuContainer'; +import {ContextMenuContext} from '../context'; +import type {ContextMenuContextType} from '../context'; import type {InspectedElement} from 'react-devtools-shared/src/frontend/types'; import type {Element} from 'react-devtools-shared/src/frontend/types'; import type {Element as ReactElement} from 'react'; @@ -91,6 +93,8 @@ export default function KeyValue({ const contextMenuTriggerRef = useRef(null); const {inspectPaths} = useContext(InspectedElementContext); + const {viewAttributeSourceFunction} = + useContext(ContextMenuContext); let isInspectable = false; let isReadOnlyBasedOnMetadata = false; @@ -268,8 +272,8 @@ export default function KeyValue({ ); + } else if (pathIsFunction && viewAttributeSourceFunction != null) { + children = ( + +