Skip to content

Commit

Permalink
ref(replay): Refactor replay tabs so they each read the context they …
Browse files Browse the repository at this point in the history
…need directly, fewer props (#55505)

I wanted to push all the context reading down into each replay tab, so
that each tab reads & can re-render only when it's data changes.
  • Loading branch information
ryan953 authored Aug 31, 2023
1 parent 8ec16b1 commit 9e9d45a
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 100 deletions.
13 changes: 5 additions & 8 deletions static/app/views/replays/detail/console/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import Placeholder from 'sentry/components/placeholder';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {t} from 'sentry/locale';
import useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
import type {BreadcrumbFrame} from 'sentry/utils/replays/types';
import ConsoleFilters from 'sentry/views/replays/detail/console/consoleFilters';
import ConsoleLogRow from 'sentry/views/replays/detail/console/consoleLogRow';
import useConsoleFilters from 'sentry/views/replays/detail/console/useConsoleFilters';
Expand All @@ -21,25 +20,23 @@ import useVirtualizedList from 'sentry/views/replays/detail/useVirtualizedList';

import useVirtualizedInspector from '../useVirtualizedInspector';

interface Props {
frames: undefined | BreadcrumbFrame[];
startTimestampMs: number;
}

// Ensure this object is created once as it is an input to
// `useVirtualizedList`'s memoization
const cellMeasurer = {
fixedWidth: true,
minHeight: 24,
};

function Console({frames, startTimestampMs}: Props) {
function Console() {
const {currentTime, currentHoverTime, replay} = useReplayContext();
const {onMouseEnter, onMouseLeave, onClickTimestamp} = useCrumbHandlers();

const startTimestampMs = replay?.getReplay()?.started_at?.getTime() ?? 0;
const frames = replay?.getConsoleFrames();

const filterProps = useConsoleFilters({frames: frames || []});
const {expandPathsRef, searchTerm, logLevel, items, setSearchTerm} = filterProps;
const clearSearchTerm = () => setSearchTerm('');
const {currentTime, currentHoverTime} = useReplayContext();

const listRef = useRef<ReactVirtualizedList>(null);

Expand Down
11 changes: 4 additions & 7 deletions static/app/views/replays/detail/domMutations/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ import NoRowRenderer from 'sentry/views/replays/detail/noRowRenderer';
import TabItemContainer from 'sentry/views/replays/detail/tabItemContainer';
import useVirtualizedList from 'sentry/views/replays/detail/useVirtualizedList';

type Props = {
replay: null | ReplayReader;
startTimestampMs: number;
};

// Ensure this object is created once as it is an input to
// `useVirtualizedList`'s memoization
const cellMeasurer = {
Expand All @@ -41,11 +36,13 @@ function useExtractedDomNodes({replay}: {replay: null | ReplayReader}) {
});
}

function DomMutations({replay, startTimestampMs}: Props) {
function DomMutations() {
const {currentTime, currentHoverTime, replay} = useReplayContext();
const {data: actions, isFetching} = useExtractedDomNodes({replay});
const {currentTime, currentHoverTime} = useReplayContext();
const {onMouseEnter, onMouseLeave, onClickTimestamp} = useCrumbHandlers();

const startTimestampMs = replay?.getReplay()?.started_at?.getTime() ?? 0;

const filterProps = useDomFilters({actions: actions || []});
const {items, setSearchTerm} = filterProps;
const clearSearchTerm = () => setSearchTerm('');
Expand Down
13 changes: 5 additions & 8 deletions static/app/views/replays/detail/errorList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import Placeholder from 'sentry/components/placeholder';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {t} from 'sentry/locale';
import useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
import type {ErrorFrame} from 'sentry/utils/replays/types';
import ErrorFilters from 'sentry/views/replays/detail/errorList/errorFilters';
import ErrorHeaderCell, {
COLUMN_COUNT,
Expand All @@ -21,21 +20,19 @@ import useVirtualizedGrid from 'sentry/views/replays/detail/useVirtualizedGrid';
const HEADER_HEIGHT = 25;
const BODY_HEIGHT = 28;

type Props = {
errorFrames: undefined | ErrorFrame[];
startTimestampMs: number;
};

const cellMeasurer = {
defaultHeight: BODY_HEIGHT,
defaultWidth: 100,
fixedHeight: true,
};

function ErrorList({errorFrames, startTimestampMs}: Props) {
const {currentTime, currentHoverTime} = useReplayContext();
function ErrorList() {
const {currentTime, currentHoverTime, replay} = useReplayContext();
const {onMouseEnter, onMouseLeave, onClickTimestamp} = useCrumbHandlers();

const errorFrames = replay?.getErrorFrames();
const startTimestampMs = replay?.getReplay().started_at.getTime() ?? 0;

const filterProps = useErrorFilters({errorFrames: errorFrames || []});
const {items: filteredItems, searchTerm, setSearchTerm} = filterProps;
const clearSearchTerm = () => setSearchTerm('');
Expand Down
48 changes: 6 additions & 42 deletions static/app/views/replays/detail/layout/focusArea.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {useReplayContext} from 'sentry/components/replays/replayContext';
import useActiveReplayTab, {TabKey} from 'sentry/utils/replays/hooks/useActiveReplayTab';
import useOrganization from 'sentry/utils/useOrganization';
import A11y from 'sentry/views/replays/detail/accessibility/index';
import Console from 'sentry/views/replays/detail/console';
import DomMutations from 'sentry/views/replays/detail/domMutations';
Expand All @@ -14,59 +12,25 @@ type Props = {};

function FocusArea({}: Props) {
const {getActiveTab} = useActiveReplayTab();
const {currentTime, currentHoverTime, replay, setCurrentTime, setCurrentHoverTime} =
useReplayContext();
const organization = useOrganization();

switch (getActiveTab()) {
case TabKey.A11Y:
return <A11y />;
case TabKey.NETWORK:
return (
<NetworkList
isNetworkDetailsSetup={Boolean(replay?.isNetworkDetailsSetup())}
networkFrames={replay?.getNetworkFrames()}
projectId={replay?.getReplay()?.project_id}
startTimestampMs={replay?.getReplay()?.started_at?.getTime() || 0}
/>
);
return <NetworkList />;
case TabKey.TRACE:
return <Trace organization={organization} replayRecord={replay?.getReplay()} />;
return <Trace />;
case TabKey.PERF:
return <PerfTable />;
case TabKey.ERRORS:
return (
<ErrorList
errorFrames={replay?.getErrorFrames()}
startTimestampMs={replay?.getReplay().started_at.getTime() ?? 0}
/>
);
return <ErrorList />;
case TabKey.DOM:
return (
<DomMutations
replay={replay}
startTimestampMs={replay?.getReplay()?.started_at?.getTime() || 0}
/>
);
return <DomMutations />;
case TabKey.MEMORY:
return (
<MemoryChart
currentTime={currentTime}
currentHoverTime={currentHoverTime}
memoryFrames={replay?.getMemoryFrames()}
setCurrentTime={setCurrentTime}
setCurrentHoverTime={setCurrentHoverTime}
startTimestampMs={replay?.getReplay()?.started_at?.getTime()}
/>
);
return <MemoryChart />;
case TabKey.CONSOLE:
default: {
return (
<Console
frames={replay?.getConsoleFrames()}
startTimestampMs={replay?.getReplay().started_at.getTime() || 0}
/>
);
return <Console />;
}
}
}
Expand Down
26 changes: 14 additions & 12 deletions static/app/views/replays/detail/memoryChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import XAxis from 'sentry/components/charts/components/xAxis';
import YAxis from 'sentry/components/charts/components/yAxis';
import EmptyMessage from 'sentry/components/emptyMessage';
import Placeholder from 'sentry/components/placeholder';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {showPlayerTime} from 'sentry/components/replays/utils';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
Expand Down Expand Up @@ -233,11 +234,6 @@ const MemoizedMemoryChart = memo(
))
);

interface MemoryChartContainerProps extends Props {
currentHoverTime: number | undefined;
currentTime: number;
}

/**
* This container is used to update echarts outside of React. `currentTime` is
* the current time of the player -- if replay is currently playing, this will
Expand All @@ -248,15 +244,15 @@ interface MemoryChartContainerProps extends Props {
* infrequently as possible, so we use React.memo and only pass in props that
* are not frequently updated.
*/
function MemoryChartContainer({
currentTime,
currentHoverTime,
startTimestampMs = 0,
...props
}: MemoryChartContainerProps) {
function MemoryChartContainer() {
const {currentTime, currentHoverTime, replay, setCurrentTime, setCurrentHoverTime} =
useReplayContext();
const chart = useRef<ReactEchartsRef>(null);
const theme = useTheme();

const memoryFrames = replay?.getMemoryFrames();
const startTimestampMs = replay?.getReplay()?.started_at?.getTime() ?? 0;

useEffect(() => {
if (!chart.current) {
return;
Expand Down Expand Up @@ -306,7 +302,13 @@ function MemoryChartContainer({
}, [currentHoverTime, startTimestampMs, theme]);

return (
<MemoizedMemoryChart ref={chart} startTimestampMs={startTimestampMs} {...props} />
<MemoizedMemoryChart
ref={chart}
memoryFrames={memoryFrames}
setCurrentHoverTime={setCurrentHoverTime}
setCurrentTime={setCurrentTime}
startTimestampMs={startTimestampMs}
/>
);
}

Expand Down
22 changes: 7 additions & 15 deletions static/app/views/replays/detail/network/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {t} from 'sentry/locale';
import {trackAnalytics} from 'sentry/utils/analytics';
import useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
import {getFrameMethod, getFrameStatus} from 'sentry/utils/replays/resourceFrame';
import type {SpanFrame} from 'sentry/utils/replays/types';
import useOrganization from 'sentry/utils/useOrganization';
import {useResizableDrawer} from 'sentry/utils/useResizableDrawer';
import useUrlParams from 'sentry/utils/useUrlParams';
Expand All @@ -30,29 +29,22 @@ const BODY_HEIGHT = 28;

const RESIZEABLE_HANDLE_HEIGHT = 90;

type Props = {
isNetworkDetailsSetup: boolean;
networkFrames: undefined | SpanFrame[];
projectId: undefined | string;
startTimestampMs: number;
};

const cellMeasurer = {
defaultHeight: BODY_HEIGHT,
defaultWidth: 100,
fixedHeight: true,
};

function NetworkList({
isNetworkDetailsSetup,
networkFrames,
projectId,
startTimestampMs,
}: Props) {
function NetworkList() {
const organization = useOrganization();
const {currentTime, currentHoverTime} = useReplayContext();
const {currentTime, currentHoverTime, replay} = useReplayContext();
const {onMouseEnter, onMouseLeave, onClickTimestamp} = useCrumbHandlers();

const isNetworkDetailsSetup = Boolean(replay?.isNetworkDetailsSetup());
const networkFrames = replay?.getNetworkFrames();
const projectId = replay?.getReplay()?.project_id;
const startTimestampMs = replay?.getReplay()?.started_at?.getTime() || 0;

const [scrollToRow, setScrollToRow] = useState<undefined | number>(undefined);

const filterProps = useNetworkFilters({networkFrames: networkFrames || []});
Expand Down
15 changes: 7 additions & 8 deletions static/app/views/replays/detail/trace/index.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import Feature from 'sentry/components/acl/feature';
import FeatureDisabled from 'sentry/components/acl/featureDisabled';
import {useReplayContext} from 'sentry/components/replays/replayContext';
import {t} from 'sentry/locale';
import type {Organization} from 'sentry/types';
import useOrganization from 'sentry/utils/useOrganization';
import Trace from 'sentry/views/replays/detail/trace/trace';
import type {ReplayRecord} from 'sentry/views/replays/types';

type Props = {
organization: Organization;
replayRecord: undefined | ReplayRecord;
};

const features = ['organizations:performance-view'];

Expand All @@ -23,7 +18,11 @@ function PerfDisabled() {
);
}

function TraceFeature({organization, replayRecord}: Props) {
function TraceFeature() {
const organization = useOrganization();
const {replay} = useReplayContext();
const replayRecord = replay?.getReplay();

return (
<Feature
features={features}
Expand Down

0 comments on commit 9e9d45a

Please sign in to comment.