Skip to content

ref: Streamline loading replays for hydration diffs in Issue Details #88028

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 31, 2025
Merged

ref: Streamline loading replays for hydration diffs in Issue Details #88028

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8fee047
ref: Streamline loading replays for hydration diffs in Issue Details
ryan953 Mar 26, 2025
f4296be
update message
ryan953 Mar 26, 2025
e21280b
extract useLoadReplayReader from ReplayLoadingState, inject instead
ryan953 Mar 26, 2025
4c55a71
Merge branch 'master' into ryan953/ref-replay-hydration-diff-loading
ryan953 Mar 26, 2025
f34e5ff
less functional, more reacty
ryan953 Mar 27, 2025
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
131 changes: 87 additions & 44 deletions static/app/components/events/eventHydrationDiff/replayDiffContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
import type {ReactNode} from 'react';
import styled from '@emotion/styled';

import NegativeSpaceContainer from 'sentry/components/container/negativeSpaceContainer';
import ErrorBoundary from 'sentry/components/errorBoundary';
import Placeholder from 'sentry/components/placeholder';
import {REPLAY_LOADING_HEIGHT} from 'sentry/components/events/eventReplay/constants';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import ArchivedReplayAlert from 'sentry/components/replays/alerts/archivedReplayAlert';
import MissingReplayAlert from 'sentry/components/replays/alerts/missingReplayAlert';
import {OpenReplayComparisonButton} from 'sentry/components/replays/breadcrumbs/openReplayComparisonButton';
import {DiffCompareContextProvider} from 'sentry/components/replays/diff/diffCompareContext';
import {ReplaySliderDiff} from 'sentry/components/replays/diff/replaySliderDiff';
import ReplayLoadingState from 'sentry/components/replays/player/replayLoadingState';
import {ReplayGroupContextProvider} from 'sentry/components/replays/replayGroupContext';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Event} from 'sentry/types/event';
import type {Group} from 'sentry/types/group';
import {getReplayDiffOffsetsFromEvent} from 'sentry/utils/replays/getDiffTimestamps';
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
import useOrganization from 'sentry/utils/useOrganization';
import {SectionKey} from 'sentry/views/issueDetails/streamline/context';
import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection';

Expand All @@ -20,54 +30,87 @@ interface Props {
}

export default function ReplayDiffContent({event, group, orgSlug, replaySlug}: Props) {
const replayContext = useLoadReplayReader({
orgSlug,
replaySlug,
});
const {fetching, replay} = replayContext;

if (fetching) {
return <Placeholder />;
function wrapInSection(render: () => ReactNode) {
return function () {
return (
<InterimSection
type={SectionKey.HYDRATION_DIFF}
title={t('Hydration Error Diff')}
>
{render()}
</InterimSection>
);
};
}
Copy link
Member

Choose a reason for hiding this comment

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

just pull this out into a component with children?

Copy link
Member Author

Choose a reason for hiding this comment

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

@scttcper it gets formatted out to more line if it's a React component :(

Copy link
Member

Choose a reason for hiding this comment

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

that seems fine


if (!replay) {
return null;
}
const organization = useOrganization();
const readerResult = useLoadReplayReader({
orgSlug: organization.slug,
replaySlug,
clipWindow: undefined,
});

const {frameOrEvent, leftOffsetMs, rightOffsetMs} = getReplayDiffOffsetsFromEvent(
replay,
event
);
return (
<InterimSection
type={SectionKey.HYDRATION_DIFF}
title={t('Hydration Error Diff')}
actions={
<OpenReplayComparisonButton
frameOrEvent={frameOrEvent}
initialLeftOffsetMs={leftOffsetMs}
initialRightOffsetMs={rightOffsetMs}
key="open-modal-button"
replay={replay}
size="xs"
surface="issue-details" // TODO: refactor once this component is used in more surfaces
>
{t('Open Diff Viewer')}
</OpenReplayComparisonButton>
}
<ReplayLoadingState
readerResult={readerResult}
renderArchived={wrapInSection(() => (
<ArchivedReplayAlert message={t('The replay for this event has been deleted.')} />
))}
renderLoading={wrapInSection(() => (
<StyledNegativeSpaceContainer data-test-id="replay-diff-loading-placeholder">
<LoadingIndicator />
</StyledNegativeSpaceContainer>
))}
renderError={wrapInSection(() => (
<MissingReplayAlert orgSlug={orgSlug} />
))}
renderMissing={wrapInSection(() => (
<MissingReplayAlert orgSlug={orgSlug} />
))}
>
<ErrorBoundary mini>
<ReplayGroupContextProvider groupId={group?.id} eventId={event.id}>
<DiffCompareContextProvider
replay={replay}
frameOrEvent={frameOrEvent}
initialLeftOffsetMs={leftOffsetMs}
initialRightOffsetMs={rightOffsetMs}
{({replay}) => {
const {frameOrEvent, leftOffsetMs, rightOffsetMs} = getReplayDiffOffsetsFromEvent(
replay,
event
);
return (
<InterimSection
type={SectionKey.HYDRATION_DIFF}
title={t('Hydration Error Diff')}
actions={
<OpenReplayComparisonButton
frameOrEvent={frameOrEvent}
initialLeftOffsetMs={leftOffsetMs}
initialRightOffsetMs={rightOffsetMs}
key="open-modal-button"
replay={replay}
size="xs"
surface="issue-details" // TODO: refactor once this component is used in more surfaces
>
{t('Open Diff Viewer')}
</OpenReplayComparisonButton>
}
>
<ReplaySliderDiff minHeight="355px" />
</DiffCompareContextProvider>
</ReplayGroupContextProvider>
</ErrorBoundary>
</InterimSection>
<ErrorBoundary mini>
<ReplayGroupContextProvider groupId={group?.id} eventId={event.id}>
<DiffCompareContextProvider
replay={replay}
frameOrEvent={frameOrEvent}
initialLeftOffsetMs={leftOffsetMs}
initialRightOffsetMs={rightOffsetMs}
>
<ReplaySliderDiff minHeight="355px" />
</DiffCompareContextProvider>
</ReplayGroupContextProvider>
</ErrorBoundary>
</InterimSection>
);
}}
</ReplayLoadingState>
);
}

const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
height: ${REPLAY_LOADING_HEIGHT}px;
margin-bottom: ${space(2)};
`;
2 changes: 1 addition & 1 deletion static/app/components/events/eventHydrationDiff/replayDiffSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function ReplayDiffSection({event, group, replayId}: Props) {
);
}

export const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
const StyledNegativeSpaceContainer = styled(NegativeSpaceContainer)`
height: ${REPLAY_LOADING_HEIGHT}px;
margin-bottom: ${space(2)};
`;
65 changes: 42 additions & 23 deletions static/app/components/replays/player/__stories__/replaySlugChooser.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {css} from '@emotion/react';

import Providers from 'sentry/components/replays/player/__stories__/providers';
import ReplayLoadingState from 'sentry/components/replays/player/replayLoadingState';
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
import useOrganization from 'sentry/utils/useOrganization';
import {useSessionStorage} from 'sentry/utils/useSessionStorage';

type Props =
Expand All @@ -14,33 +16,50 @@ export default function ReplaySlugChooser(props: Props) {

const [replaySlug, setReplaySlug] = useSessionStorage('stories:replaySlug', '');

let content = null;
if (replaySlug) {
if (children) {
content = (
<ReplayLoadingState replaySlug={replaySlug}>
const input = (
<input
defaultValue={replaySlug}
onChange={event => {
setReplaySlug(event.target.value);
}}
placeholder="Paste a replaySlug"
css={css`
font-variant-numeric: tabular-nums;
`}
size={34}
/>
);

if (replaySlug && children) {
function Content() {
const organization = useOrganization();
const readerResult = useLoadReplayReader({
orgSlug: organization.slug,
replaySlug,
clipWindow: undefined,
});
return (
<ReplayLoadingState readerResult={readerResult}>
{({replay}) => <Providers replay={replay}>{children}</Providers>}
</ReplayLoadingState>
);
} else if (render) {
content = render(replaySlug);
}
return (
<Fragment>
{input}
<Content />
</Fragment>
);
}

return (
<Fragment>
<input
defaultValue={replaySlug}
onChange={event => {
setReplaySlug(event.target.value);
}}
placeholder="Paste a replaySlug"
css={css`
font-variant-numeric: tabular-nums;
`}
size={34}
/>
{content}
</Fragment>
);
if (replaySlug && render) {
return (
<Fragment>
{input}
{render(replaySlug)}
Copy link
Member

Choose a reason for hiding this comment

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

imo it's kind of confusing to have two different rendering methods in this component -- I don't really know how they're used but what if they were two components ReplaySlugChooser and ReplaySlugChooserWithLoaderStuff

Copy link
Member Author

Choose a reason for hiding this comment

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

i'll pass back over this chooser and clean it up.

it's used by some /stories/ files so people can load up real reply data and see the video components working together, so the two interfaces kind of optimize for keeping the stories readable right now.

</Fragment>
);
}

return null;
}
21 changes: 3 additions & 18 deletions static/app/components/replays/player/replayLoadingState.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,28 @@ import type {ReactNode} from 'react';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import ArchivedReplayAlert from 'sentry/components/replays/alerts/archivedReplayAlert';
import MissingReplayAlert from 'sentry/components/replays/alerts/missingReplayAlert';
import useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
import type useLoadReplayReader from 'sentry/utils/replays/hooks/useLoadReplayReader';
import type ReplayReader from 'sentry/utils/replays/replayReader';
import useOrganization from 'sentry/utils/useOrganization';

type ClipWindow = {
// When to stop the replay, given it continues into that time
endTimestampMs: number;

// When to start the replay, given its start time is early enough
startTimestampMs: number;
};

type ReplayReaderResult = ReturnType<typeof useLoadReplayReader>;

export default function ReplayLoadingState({
children,
replaySlug,
clipWindow,
readerResult,
renderArchived,
renderError,
renderLoading,
renderMissing,
}: {
children: (props: {replay: ReplayReader}) => ReactNode;
replaySlug: string;
clipWindow?: ClipWindow;
readerResult: ReplayReaderResult;
renderArchived?: (results: ReplayReaderResult) => ReactNode;
renderError?: (results: ReplayReaderResult) => ReactNode;
renderLoading?: (results: ReplayReaderResult) => ReactNode;
renderMissing?: (results: ReplayReaderResult) => ReactNode;
}) {
const organization = useOrganization();
const readerResult = useLoadReplayReader({
orgSlug: organization.slug,
replaySlug,
clipWindow,
});

if (readerResult.fetchError) {
return renderError ? (
Expand Down
Loading