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

feat[devtools]: display Forget badge for the relevant components #27709

Merged
merged 1 commit into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2765,13 +2765,15 @@ describe('InspectedElement', () => {
expect(inspectedElement.owners).toMatchInlineSnapshot(`
[
{
"compiledWithForget": false,
"displayName": "Child",
"hocDisplayNames": null,
"id": 3,
"key": null,
"type": 5,
},
{
"compiledWithForget": false,
"displayName": "App",
"hocDisplayNames": null,
"id": 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,7 @@ describe('ProfilingCache', () => {
"timestamp": 0,
"updaters": [
{
"compiledWithForget": false,
"displayName": "render()",
"hocDisplayNames": null,
"id": 1,
Expand Down Expand Up @@ -1010,6 +1011,7 @@ describe('ProfilingCache', () => {
"timestamp": 0,
"updaters": [
{
"compiledWithForget": false,
"displayName": "render()",
"hocDisplayNames": null,
"id": 1,
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 @@ -424,7 +424,10 @@ export function getInternalReactConstants(version: string): {
}

// NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods
function getDisplayNameForFiber(fiber: Fiber): string | null {
function getDisplayNameForFiber(
fiber: Fiber,
shouldSkipForgetCheck: boolean = false,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to do this without adding a boolean argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, probably. Without recursion, but with some auxiliary variables. Why do you want to remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

The code is unnecessarily complex here with the recursion though? It's harder to reason about

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is unnecessarily complex here with the recursion though?

I don't think so, but I can rewrite it, I don't have a strong opinion on it.

I am kinda surprised that this is the only thing we are discussing in this PR.

): string | null {
const {elementType, type, tag} = fiber;

let resolvedType = type;
Expand All @@ -433,6 +436,18 @@ export function getInternalReactConstants(version: string): {
}

let resolvedContext: any = null;
// $FlowFixMe[incompatible-type] fiber.updateQueue is mixed
if (!shouldSkipForgetCheck && fiber.updateQueue?.memoCache != null) {
const displayNameWithoutForgetWrapper = getDisplayNameForFiber(
fiber,
true,
);
if (displayNameWithoutForgetWrapper == null) {
return null;
}

return `Forget(${displayNameWithoutForgetWrapper})`;
}

switch (tag) {
case CacheComponent:
Expand Down
14 changes: 2 additions & 12 deletions packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration';
import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils';
import {backendToFrontendSerializedElementMapper} from 'react-devtools-shared/src/utils';
import Store from 'react-devtools-shared/src/devtools/store';
import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';
import ElementPollingCancellationError from 'react-devtools-shared/src/errors/ElementPollingCancellationError';
Expand Down Expand Up @@ -266,17 +266,7 @@ export function convertInspectedElementBackendToFrontend(
owners:
owners === null
? null
: owners.map(owner => {
const [displayName, hocDisplayNames] = separateDisplayNameAndHOCs(
owner.displayName,
owner.type,
);
return {
...owner,
displayName,
hocDisplayNames,
};
}),
: owners.map(backendToFrontendSerializedElementMapper),
context: hydrateHelper(context),
hooks: hydrateHelper(hooks),
props: hydrateHelper(props),
Expand Down
2 changes: 2 additions & 0 deletions packages/react-devtools-shared/src/devtools/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = {
'--color-error-border': 'hsl(0, 100%, 92%)',
'--color-error-text': '#ff0000',
'--color-expand-collapse-toggle': '#777d88',
'--color-forget-badge': '#2683E2',
'--color-link': '#0000ff',
'--color-modal-background': 'rgba(255, 255, 255, 0.75)',
'--color-bridge-version-npm-background': '#eff0f1',
Expand Down Expand Up @@ -221,6 +222,7 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any, ...} = {
'--color-error-border': '#900',
'--color-error-text': '#f55',
'--color-expand-collapse-toggle': '#8f949d',
'--color-forget-badge': '#2683E2',
'--color-link': '#61dafb',
'--color-modal-background': 'rgba(0, 0, 0, 0.75)',
'--color-bridge-version-npm-background': 'rgba(0, 0, 0, 0.25)',
Expand Down
11 changes: 8 additions & 3 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import {ElementTypeRoot} from '../frontend/types';
import {
getSavedComponentFilters,
setSavedComponentFilters,
separateDisplayNameAndHOCs,
shallowDiffers,
utfDecodeStringWithRanges,
parseElementDisplayNameFromBackend,
} from '../utils';
import {localStorageGetItem, localStorageSetItem} from '../storage';
import {__DEBUG__} from '../constants';
Expand Down Expand Up @@ -1033,6 +1033,7 @@ export default class Store extends EventEmitter<{
parentID: 0,
type,
weight: 0,
compiledWithForget: false,
});

haveRootsChanged = true;
Expand Down Expand Up @@ -1071,8 +1072,11 @@ export default class Store extends EventEmitter<{

parentElement.children.push(id);

const [displayNameWithoutHOCs, hocDisplayNames] =
separateDisplayNameAndHOCs(displayName, type);
const {
formattedDisplayName: displayNameWithoutHOCs,
hocDisplayNames,
compiledWithForget,
} = parseElementDisplayNameFromBackend(displayName, type);

const element: Element = {
children: [],
Expand All @@ -1087,6 +1091,7 @@ export default class Store extends EventEmitter<{
parentID,
type,
weight: 1,
compiledWithForget,
};

this._idToElement.set(id, element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,3 @@
font-family: var(--font-family-monospace);
font-size: var(--font-size-monospace-small);
}

.ExtraLabel {
font-family: var(--font-family-monospace);
font-size: var(--font-size-monospace-small);
color: var(--color-component-badge-count);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,36 +8,14 @@
*/

import * as React from 'react';
import {Fragment} from 'react';
import styles from './Badge.css';

import type {ElementType} from 'react-devtools-shared/src/frontend/types';
import styles from './Badge.css';

type Props = {
className?: string,
hocDisplayNames: Array<string> | null,
type: ElementType,
children: React$Node,
};

export default function Badge({
className,
hocDisplayNames,
type,
children,
}: Props): React.Node {
if (hocDisplayNames === null || hocDisplayNames.length === 0) {
return null;
}

const totalBadgeCount = hocDisplayNames.length;

return (
<Fragment>
<div className={`${styles.Badge} ${className || ''}`}>{children}</div>
{totalBadgeCount > 1 && (
<div className={styles.ExtraLabel}>+{totalBadgeCount - 1}</div>
)}
</Fragment>
);
export default function Badge({className = '', children}: Props): React.Node {
return <div className={`${styles.Badge} ${className}`}>{children}</div>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
color: var(--color-expand-collapse-toggle);
}

.Badge {
.BadgesBlock {
margin-left: 0.25rem;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
import * as React from 'react';
import {Fragment, useContext, useMemo, useState} from 'react';
import Store from 'react-devtools-shared/src/devtools/store';
import Badge from './Badge';
import ButtonIcon from '../ButtonIcon';
import {createRegExp} from '../utils';
import {TreeDispatcherContext, TreeStateContext} from './TreeContext';
import {SettingsContext} from '../Settings/SettingsContext';
import {StoreContext} from '../context';
import {useSubscription} from '../hooks';
import {logEvent} from 'react-devtools-shared/src/Logger';
import IndexableElementBadges from './IndexableElementBadges';
import IndexableDisplayName from './IndexableDisplayName';

import type {ItemData} from './Tree';
import type {Element as ElementType} from 'react-devtools-shared/src/frontend/types';
Expand Down Expand Up @@ -121,7 +121,7 @@ export default function Element({data, index, style}: Props): React.Node {
hocDisplayNames,
isStrictModeNonCompliant,
key,
type,
compiledWithForget,
} = element;

// Only show strict mode non-compliance badges for top level elements.
Expand Down Expand Up @@ -155,11 +155,11 @@ export default function Element({data, index, style}: Props): React.Node {
// We must use padding rather than margin/left because of the selected background color.
transform: `translateX(calc(${depth} * var(--indentation-size)))`,
}}>
{ownerID === null ? (
{ownerID === null && (
<ExpandCollapseToggle element={element} store={store} />
) : null}
)}

<DisplayName displayName={displayName} id={((id: any): number)} />
<IndexableDisplayName displayName={displayName} id={id} />

{key && (
<Fragment>
Expand All @@ -174,14 +174,12 @@ export default function Element({data, index, style}: Props): React.Node {
</Fragment>
)}

{hocDisplayNames !== null && hocDisplayNames.length > 0 ? (
<Badge
className={styles.Badge}
hocDisplayNames={hocDisplayNames}
type={type}>
<DisplayName displayName={hocDisplayNames[0]} id={id} />
</Badge>
) : null}
<IndexableElementBadges
hocDisplayNames={hocDisplayNames}
compiledWithForget={compiledWithForget}
elementID={id}
className={styles.BadgesBlock}
/>

{showInlineWarningsAndErrors && errorCount > 0 && (
<Icon
Expand Down Expand Up @@ -262,47 +260,3 @@ function ExpandCollapseToggle({element, store}: ExpandCollapseToggleProps) {
</div>
);
}

type DisplayNameProps = {
displayName: string | null,
id: number,
};

function DisplayName({displayName, id}: DisplayNameProps) {
const {searchIndex, searchResults, searchText} = useContext(TreeStateContext);
const isSearchResult = useMemo(() => {
return searchResults.includes(id);
}, [id, searchResults]);
const isCurrentResult =
searchIndex !== null && id === searchResults[searchIndex];

if (!isSearchResult || displayName === null) {
return displayName;
}

const match = createRegExp(searchText).exec(displayName);

if (match === null) {
return displayName;
}

const startIndex = match.index;
const stopIndex = startIndex + match[0].length;

const children = [];
if (startIndex > 0) {
children.push(<span key="begin">{displayName.slice(0, startIndex)}</span>);
}
children.push(
<mark
key="middle"
className={isCurrentResult ? styles.CurrentHighlight : styles.Highlight}>
{displayName.slice(startIndex, stopIndex)}
</mark>,
);
if (stopIndex < displayName.length) {
children.push(<span key="end">{displayName.slice(stopIndex)}</span>);
}

return children;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
.Root {
display: inline-flex;
align-items: center;
}

.Root *:not(:first-child) {
margin-left: 0.25rem;
}

.ExtraLabel {
font-family: var(--font-family-monospace);
font-size: var(--font-size-monospace-small);
color: var(--color-component-badge-count);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import * as React from 'react';

import Badge from './Badge';
import ForgetBadge from './ForgetBadge';

import styles from './ElementBadges.css';

type Props = {
hocDisplayNames: Array<string> | null,
compiledWithForget: boolean,
className?: string,
};

export default function ElementBadges({
compiledWithForget,
hocDisplayNames,
className = '',
}: Props): React.Node {
if (
!compiledWithForget &&
(hocDisplayNames == null || hocDisplayNames.length === 0)
) {
return null;
}

return (
<div className={`${styles.Root} ${className}`}>
{compiledWithForget && <ForgetBadge indexable={false} />}

{hocDisplayNames != null && hocDisplayNames.length > 0 && (
<Badge>{hocDisplayNames[0]}</Badge>
)}

{hocDisplayNames != null && hocDisplayNames.length > 1 && (
<div className={styles.ExtraLabel}>+{hocDisplayNames.length - 1}</div>
)}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.Root {
background-color: var(--color-forget-badge);
}
Loading