Browse Source

chore(replay): Remove object inspector (#78236)

- Switch from ObjectInspector to StructuredEventData in the console tab
- Removes ObjectInspector and react-inspector!!

Before:
<img width="756" alt="image"
src="https://github.com/user-attachments/assets/00af64b6-6df2-48b5-bc81-b39534492bc2">

After:
<img width="756" alt="image"
src="https://github.com/user-attachments/assets/a353da7a-e9d4-4cf7-9daf-8d4deb38aac1">

Closes https://github.com/getsentry/sentry/issues/70042
Catherine Lee 5 months ago
parent
commit
61dc82ada0

+ 0 - 1
package.json

@@ -54,7 +54,6 @@
     "@react-types/shared": "^3.24.1",
     "@rsdoctor/webpack-plugin": "0.4.4",
     "@sentry-internal/global-search": "^1.0.0",
-    "@sentry-internal/react-inspector": "6.0.1-4",
     "@sentry-internal/rrweb": "2.26.0",
     "@sentry-internal/rrweb-player": "2.26.0",
     "@sentry-internal/rrweb-snapshot": "2.26.0",

+ 0 - 101
static/app/components/objectInspector.tsx

@@ -1,101 +0,0 @@
-import type {ComponentProps, MouseEvent} from 'react';
-import {useMemo} from 'react';
-import {useTheme} from '@emotion/react';
-import styled from '@emotion/styled';
-import {
-  chromeDark,
-  chromeLight,
-  ObjectInspector as OrigObjectInspector,
-} from '@sentry-internal/react-inspector';
-
-import {CopyToClipboardButton} from 'sentry/components/copyToClipboardButton';
-import ConfigStore from 'sentry/stores/configStore';
-import {useLegacyStore} from 'sentry/stores/useLegacyStore';
-import {space} from 'sentry/styles/space';
-
-type Props = Omit<ComponentProps<typeof OrigObjectInspector>, 'theme'> & {
-  onCopy?: (copiedCode: string) => void;
-  showCopyButton?: boolean;
-  theme?: Record<string, any>;
-};
-
-/**
- * @deprecated use `StructuredEventData` or `StructuredData` instead.
- */
-function ObjectInspector({data, onCopy, showCopyButton, theme, ...props}: Props) {
-  const config = useLegacyStore(ConfigStore);
-  const emotionTheme = useTheme();
-  const isDark = config.theme === 'dark';
-
-  const INSPECTOR_THEME = useMemo(
-    () => ({
-      ...(isDark ? chromeDark : chromeLight),
-
-      // Reset some theme values
-      BASE_COLOR: 'inherit',
-      ERROR_COLOR: emotionTheme.red400,
-      TREENODE_FONT_FAMILY: emotionTheme.text.familyMono,
-      TREENODE_FONT_SIZE: 'inherit',
-      TREENODE_LINE_HEIGHT: 'inherit',
-      BASE_BACKGROUND_COLOR: 'none',
-      ARROW_FONT_SIZE: '10px',
-
-      OBJECT_PREVIEW_OBJECT_MAX_PROPERTIES: 1,
-      ...theme,
-    }),
-    [isDark, theme, emotionTheme.red400, emotionTheme.text]
-  );
-
-  const inspector = (
-    <OrigObjectInspector
-      data={data}
-      // @ts-expect-error
-      theme={INSPECTOR_THEME}
-      {...props}
-    />
-  );
-  if (showCopyButton) {
-    return (
-      <Wrapper>
-        <StyledCopyButton
-          borderless
-          iconSize="xs"
-          onCopy={onCopy}
-          size="xs"
-          text={JSON.stringify(data, null, '\t')}
-        />
-        <InspectorWrapper>{inspector}</InspectorWrapper>
-      </Wrapper>
-    );
-  }
-
-  return inspector;
-}
-
-const InspectorWrapper = styled('div')`
-  margin-right: ${space(4)};
-`;
-
-const Wrapper = styled('div')`
-  position: relative;
-
-  /*
-  We need some minimum vertical height so the copy button has room.
-  But don't try to use min-height because then whitespace would be inconsistent.
-  */
-  padding-bottom: ${space(1.5)};
-`;
-
-const StyledCopyButton = styled(CopyToClipboardButton)`
-  position: absolute;
-  top: 0;
-  right: ${space(0.5)};
-`;
-
-export type OnExpandCallback = (
-  path: string,
-  expandedState: Record<string, boolean>,
-  event: MouseEvent<HTMLDivElement>
-) => void;
-
-export default ObjectInspector;

+ 3 - 2
static/app/components/replays/breadcrumbs/breadcrumbItem.tsx

@@ -44,6 +44,7 @@ import useOrganization from 'sentry/utils/useOrganization';
 import useProjectFromSlug from 'sentry/utils/useProjectFromSlug';
 import IconWrapper from 'sentry/views/replays/detail/iconWrapper';
 import TimestampButton from 'sentry/views/replays/detail/timestampButton';
+import type {OnExpandCallback} from 'sentry/views/replays/detail/useVirtualizedInspector';
 
 type MouseCallback = (frame: ReplayFrame, nodeId?: number) => void;
 
@@ -52,7 +53,7 @@ const FRAMES_WITH_BUTTONS = ['replay.hydrate-error'];
 interface Props {
   frame: ReplayFrame;
   onClick: null | MouseCallback;
-  onInspectorExpanded: (path: string, expandedState: Record<string, boolean>) => void;
+  onInspectorExpanded: OnExpandCallback;
   onMouseEnter: MouseCallback;
   onMouseLeave: MouseCallback;
   startTimestampMs: number;
@@ -236,7 +237,7 @@ function WebVitalData({
 }: {
   expandPaths: string[] | undefined;
   frame: WebVitalFrame;
-  onInspectorExpanded: (path: string, expandedState: Record<string, boolean>) => void;
+  onInspectorExpanded: OnExpandCallback;
   onMouseEnter: MouseCallback;
   onMouseLeave: MouseCallback;
   selectors: Map<number, string> | undefined;

+ 2 - 2
static/app/components/replays/preferences/replayPreferenceDropdown.stories.tsx

@@ -1,6 +1,5 @@
 import {Fragment} from 'react';
 
-import ObjectInspector from 'sentry/components/objectInspector';
 import ReplayPreferenceDropdown from 'sentry/components/replays/preferences/replayPreferenceDropdown';
 import {
   LocalStorageReplayPreferences,
@@ -9,6 +8,7 @@ import {
 } from 'sentry/components/replays/preferences/replayPreferences';
 import JSXNode from 'sentry/components/stories/jsxNode';
 import SideBySide from 'sentry/components/stories/sideBySide';
+import StructuredEventData from 'sentry/components/structuredEventData';
 import storyBook from 'sentry/stories/storyBook';
 import {
   ReplayPreferencesContextProvider,
@@ -98,5 +98,5 @@ export default storyBook(ReplayPreferenceDropdown, story => {
 
 function DebugReplayPrefsState() {
   const [prefs] = useReplayPrefs();
-  return <ObjectInspector data={prefs} expandLevel={1} />;
+  return <StructuredEventData data={prefs} maxDefaultDepth={1} forceDefaultExpand />;
 }

+ 1 - 1
static/app/views/replays/detail/breadcrumbs/useBreadcrumbFilters.tsx

@@ -103,7 +103,7 @@ const FILTERS = {
 function useBreadcrumbFilters({frames}: Options): Return {
   const {setFilter, query} = useFiltersInLocationQuery<FilterFields>();
 
-  // Keep a reference of object paths that are expanded (via <ObjectInspector>)
+  // Keep a reference of object paths that are expanded (via <StructuredEventData>)
   // by log row, so they they can be restored as the Console pane is scrolling.
   // Due to virtualization, components can be unmounted as the user scrolls, so
   // state needs to be remembered.

+ 6 - 3
static/app/views/replays/detail/console/consoleLogRow.tsx

@@ -12,17 +12,20 @@ import type useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
 import type {BreadcrumbFrame, ConsoleFrame} from 'sentry/utils/replays/types';
 import MessageFormatter from 'sentry/views/replays/detail/console/messageFormatter';
 import TimestampButton from 'sentry/views/replays/detail/timestampButton';
-import type {OnDimensionChange} from 'sentry/views/replays/detail/useVirtualizedInspector';
 
 interface Props extends ReturnType<typeof useCrumbHandlers> {
   currentHoverTime: number | undefined;
   currentTime: number;
   frame: BreadcrumbFrame;
   index: number;
+  onDimensionChange: (
+    index: number,
+    path: string,
+    expandedState: Record<string, boolean>
+  ) => void;
   startTimestampMs: number;
   style: CSSProperties;
   expandPaths?: string[];
-  onDimensionChange?: OnDimensionChange;
 }
 
 export default function ConsoleLogRow({
@@ -39,7 +42,7 @@ export default function ConsoleLogRow({
   style,
 }: Props) {
   const handleDimensionChange = useCallback(
-    (path, expandedState, e) => onDimensionChange?.(index, path, expandedState, e),
+    (path, expandedState) => onDimensionChange?.(index, path, expandedState),
     [onDimensionChange, index]
   );
 

+ 32 - 12
static/app/views/replays/detail/console/format.tsx

@@ -20,38 +20,43 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 import {Fragment} from 'react';
+import styled from '@emotion/styled';
 
-import type {OnExpandCallback} from 'sentry/components/objectInspector';
-import ObjectInspector from 'sentry/components/objectInspector';
+import StructuredEventData from 'sentry/components/structuredEventData';
+import type {OnExpandCallback} from 'sentry/views/replays/detail/useVirtualizedInspector';
 
 const formatRegExp = /%[csdj%]/g;
-
 interface FormatProps {
   args: any[];
+  onExpand: OnExpandCallback;
   expandPaths?: string[];
-  onExpand?: OnExpandCallback;
 }
 
 /**
  * Based on node's `util.format()`, returns a formatted "string" using the
  * first argument as a printf-like format string which can contain zero or more
- * format specifiers. Uses `<ObjectInspector>` to print objects.
+ * format specifiers. Uses `<StructuredEventData>` to print objects.
  *
  * %c is ignored for now
  */
 export default function Format({onExpand, expandPaths, args}: FormatProps) {
+  const onToggleExpand = (expandedPaths, path) => {
+    onExpand(path, Object.fromEntries(expandedPaths.map(item => [item, true])));
+  };
   const f = args[0];
 
   if (typeof f !== 'string') {
     const objects: any[] = [];
     for (let i = 0; i < args.length; i++) {
       objects.push(
-        <ObjectInspector
-          key={i}
-          data={args[i]}
-          expandPaths={expandPaths}
-          onExpand={onExpand}
-        />
+        <Wrapper key={i}>
+          <StructuredEventData
+            key={i}
+            data={args[i]}
+            initialExpandedPaths={expandPaths ?? []}
+            onToggleExpand={onToggleExpand}
+          />
+        </Wrapper>
       );
     }
     return <Fragment>{objects}</Fragment>;
@@ -136,10 +141,25 @@ export default function Format({onExpand, expandPaths, args}: FormatProps) {
     } else {
       pieces.push(' ');
       pieces.push(
-        <ObjectInspector key={i} data={x} expandPaths={expandPaths} onExpand={onExpand} />
+        <Wrapper key={i}>
+          <StructuredEventData
+            key={i}
+            data={x}
+            initialExpandedPaths={expandPaths ?? []}
+            onToggleExpand={onToggleExpand}
+          />
+        </Wrapper>
       );
     }
   }
 
   return <Fragment>{pieces}</Fragment>;
 }
+
+const Wrapper = styled('div')`
+  pre {
+    margin: 0;
+    background: none;
+    font-size: inherit;
+  }
+`;

+ 1 - 1
static/app/views/replays/detail/console/index.tsx

@@ -75,7 +75,7 @@ function Console() {
         cache={cache}
         columnIndex={0}
         // Set key based on filters, otherwise we can have odd expand/collapse state
-        // with <ObjectInspector> when filtering
+        // with <StructuredEventData> when filtering
         key={`${searchTerm}-${logLevel.join(',')}-${key}`}
         parent={parent}
         rowIndex={index}

+ 26 - 24
static/app/views/replays/detail/console/messageFormatter.spec.tsx

@@ -1,7 +1,7 @@
 import {ReplayConsoleFrameFixture} from 'sentry-fixture/replay/replayBreadcrumbFrameData';
 import {ReplayRecordFixture} from 'sentry-fixture/replayRecord';
 
-import {render, screen} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import {BreadcrumbLevelType} from 'sentry/types/breadcrumbs';
 import hydrateBreadcrumbs from 'sentry/utils/replays/hydrateBreadcrumbs';
@@ -21,7 +21,7 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('This is a test')).toBeInTheDocument();
   });
@@ -41,7 +41,7 @@ describe('MessageFormatter', () => {
     // When the type is narrowed to `ConsoleFrame` the `data` field is forced to exist.
     delete frame.data;
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('This is only a test')).toBeInTheDocument();
   });
@@ -59,13 +59,13 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    const {container} = render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('test 1 false')).toBeInTheDocument();
-    expect(screen.getByText('{}')).toBeInTheDocument();
+    expect(container).toHaveTextContent('{}');
   });
 
-  it('Should print console message correctly when it is an Error object', () => {
+  it('Should print console message correctly when it is an Error object', async function () {
     const [frame] = hydrateBreadcrumbs(ReplayRecordFixture(), [
       ReplayConsoleFrameFixture({
         data: {
@@ -78,8 +78,10 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
+    expect(screen.getByText('1 item')).toBeInTheDocument();
+    await userEvent.click(screen.getByRole('button', {name: '1 item'}));
     expect(screen.getByText('this is my error message')).toBeInTheDocument();
   });
 
@@ -95,12 +97,12 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    const {container} = render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
-    expect(screen.getByText('{}')).toBeInTheDocument();
+    expect(container).toHaveTextContent('{}');
   });
 
-  it('Should style "%c" placeholder and print the console message correctly', () => {
+  it('Should style "%c" placeholder and print the console message correctly', async function () {
     const [frame] = hydrateBreadcrumbs(ReplayRecordFixture(), [
       ReplayConsoleFrameFixture({
         data: {
@@ -120,18 +122,19 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    const {container} = render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     const styledEl = screen.getByText('prev state');
     expect(styledEl).toBeInTheDocument();
     expect(styledEl).toHaveStyle('color: #9E9E9E;');
     expect(styledEl).toHaveStyle('font-weight: bold;');
     expect(styledEl).not.toHaveStyle('background-image: url(foo);');
-    expect(screen.getByText('cart')).toBeInTheDocument();
-    expect(screen.getByText('Array(0)')).toBeInTheDocument();
+    expect(screen.getByText('1 item')).toBeInTheDocument();
+    await userEvent.click(screen.getByRole('button', {name: '1 item'}));
+    expect(container).toHaveTextContent('cart: []');
   });
 
-  it('Should print arrays correctly', () => {
+  it('Should print arrays correctly', async function () {
     const [frame] = hydrateBreadcrumbs(ReplayRecordFixture(), [
       ReplayConsoleFrameFixture({
         data: {
@@ -144,14 +147,13 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('test')).toBeInTheDocument();
-    expect(screen.getByText('(2)')).toBeInTheDocument();
-    // expect(screen.getByText('[')).toBeInTheDocument();
-    expect(screen.getByText('"foo"')).toBeInTheDocument();
-    expect(screen.getByText('"bar"')).toBeInTheDocument();
-    // expect(screen.getByText(']')).toBeInTheDocument();
+    expect(screen.getByText('2 items')).toBeInTheDocument();
+    await userEvent.click(screen.getByRole('button', {name: '2 items'}));
+    expect(screen.getByText('foo')).toBeInTheDocument();
+    expect(screen.getByText('bar')).toBeInTheDocument();
   });
 
   it('Should print literal %', () => {
@@ -167,7 +169,7 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('This is a literal 100%')).toBeInTheDocument();
   });
@@ -185,7 +187,7 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('Unbound placeholder %s')).toBeInTheDocument();
   });
@@ -203,7 +205,7 @@ describe('MessageFormatter', () => {
       }),
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('Placeholder myPlaceholder with 100%')).toBeInTheDocument();
   });
@@ -218,7 +220,7 @@ describe('MessageFormatter', () => {
       },
     ]);
 
-    render(<MessageFormatter frame={frame} />);
+    render(<MessageFormatter frame={frame} onExpand={() => {}} />);
 
     expect(screen.getByText('cypress custom breadcrumb')).toBeInTheDocument();
   });

+ 10 - 3
static/app/views/replays/detail/console/messageFormatter.tsx

@@ -1,13 +1,13 @@
-import type {OnExpandCallback} from 'sentry/components/objectInspector';
 import {defined} from 'sentry/utils';
 import type {BreadcrumbFrame, ConsoleFrame} from 'sentry/utils/replays/types';
 import {isConsoleFrame} from 'sentry/utils/replays/types';
 import Format from 'sentry/views/replays/detail/console/format';
+import type {OnExpandCallback} from 'sentry/views/replays/detail/useVirtualizedInspector';
 
 interface Props {
   frame: BreadcrumbFrame;
+  onExpand: OnExpandCallback;
   expandPaths?: string[];
-  onExpand?: OnExpandCallback;
 }
 
 // There is a special case where `console.error()` is called with an Error object.
@@ -71,7 +71,14 @@ export default function MessageFormatter({frame, expandPaths, onExpand}: Props)
       // Some browsers won't allow you to write to error properties
     }
 
-    return <Format expandPaths={expandPaths} onExpand={onExpand} args={[fakeError]} />;
+    // An Error object has non enumerable attributes that we want <StructuredEventData> to print
+    const fakeErrorObject = JSON.parse(
+      JSON.stringify(fakeError, Object.getOwnPropertyNames(fakeError))
+    );
+
+    return (
+      <Format expandPaths={expandPaths} onExpand={onExpand} args={[fakeErrorObject]} />
+    );
   }
 
   return (

Some files were not shown because too many files changed in this diff