Skip to content

Commit 23e29f8

Browse files
TkDodoTkDodo
authored
ref: make VersionHoverCard lazy (#95979)
previously, it was loading its content immediately upon rendering, because `body` was a function that was executed immediately, not a react component that might not get rendered. So even if the card content was not rendered by `<HoverCard>`, the body still executed, meaning API calls were made By moving the body to a functional component, we ensure that the body is only rendered, and thus requests only being fired, once the HoverCard actually becomes visible before: fetching release details happens on page load, which means unnecessary requests if the `HoverCard` is never used. On the plus side, the Dialog opens instantly. https://github.com/user-attachments/assets/ef81354f-01fe-474a-a28d-05357203fc29 after: no fetching on page load - it happens when the user hovers the card for the first time, and we can see a short pending state while data comes in: https://github.com/user-attachments/assets/f96535a2-cac9-4e7c-bf16-31c7296b4d24
1 parent 48f981a commit 23e29f8

File tree

2 files changed

+109
-118
lines changed

2 files changed

+109
-118
lines changed

‎static/app/components/events/eventTags/eventTagsTree.spec.tsx

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -129,19 +129,6 @@ describe('EventTagsTree', function () {
129129
it('renders release tag differently', async function () {
130130
const releaseVersion = 'v1.0';
131131

132-
const reposRequest = MockApiClient.addMockResponse({
133-
url: `/organizations/${organization.slug}/repos/`,
134-
body: [],
135-
});
136-
const releasesRequest = MockApiClient.addMockResponse({
137-
url: `/projects/${organization.slug}/${project.slug}/releases/${releaseVersion}/`,
138-
body: [],
139-
});
140-
const deploysRequest = MockApiClient.addMockResponse({
141-
url: `/organizations/${organization.slug}/releases/${releaseVersion}/deploys/`,
142-
body: [],
143-
});
144-
145132
const releaseEvent = EventFixture({
146133
tags: [{key: 'release', value: releaseVersion}],
147134
});
@@ -160,9 +147,6 @@ describe('EventTagsTree', function () {
160147
expect(anchorLink.href).toContain(
161148
`/mock-pathname/?rd=show&rdRelease=${releaseVersion}&rdSource=release-version-link`
162149
);
163-
expect(reposRequest).toHaveBeenCalled();
164-
expect(releasesRequest).toHaveBeenCalled();
165-
expect(deploysRequest).toHaveBeenCalled();
166150
const dropdown = screen.getByLabelText('Tag Actions Menu');
167151
await userEvent.click(dropdown);
168152
expect(screen.getByLabelText('View this release')).toBeInTheDocument();

‎static/app/components/versionHoverCard.tsx

Lines changed: 109 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,13 @@ import {useRelease} from 'sentry/utils/useRelease';
2525
import {useRepositories} from 'sentry/utils/useRepositories';
2626
import {parseVersion} from 'sentry/utils/versions/parseVersion';
2727

28-
interface Props extends React.ComponentProps<typeof Hovercard> {
28+
interface BodyProps {
2929
organization: Organization;
3030
projectSlug: string;
3131
releaseVersion: string;
3232
}
3333

34-
function VersionHoverCard({
35-
organization,
36-
projectSlug,
37-
releaseVersion,
38-
...hovercardProps
39-
}: Props) {
34+
function VersionHoverCardBody({organization, releaseVersion, projectSlug}: BodyProps) {
4035
const {
4136
data: repositories,
4237
isPending: isRepositoriesLoading,
@@ -62,22 +57,19 @@ function VersionHoverCard({
6257

6358
function getRepoLink() {
6459
const orgSlug = organization.slug;
65-
return {
66-
header: null,
67-
body: (
68-
<ConnectRepo>
69-
<h5>{t('Releases are better with commit data!')}</h5>
70-
<p>
71-
{t(
72-
'Connect a repository to see commit info, files changed, and authors involved in future releases.'
73-
)}
74-
</p>
75-
<LinkButton to={`/settings/${orgSlug}/repos/`} priority="primary">
76-
{t('Connect a repository')}
77-
</LinkButton>
78-
</ConnectRepo>
79-
),
80-
};
60+
return (
61+
<ConnectRepo>
62+
<h5>{t('Releases are better with commit data!')}</h5>
63+
<p>
64+
{t(
65+
'Connect a repository to see commit info, files changed, and authors involved in future releases.'
66+
)}
67+
</p>
68+
<LinkButton to={`/settings/${orgSlug}/repos/`} priority="primary">
69+
{t('Connect a repository')}
70+
</LinkButton>
71+
</ConnectRepo>
72+
);
8173
}
8274

8375
const authors = useMemo(
@@ -95,7 +87,7 @@ function VersionHoverCard({
9587

9688
function getBody() {
9789
if (release === undefined || !defined(deploys)) {
98-
return {header: null, body: null};
90+
return null;
9991
}
10092

10193
const parsedVersion = parseVersion(releaseVersion);
@@ -106,93 +98,108 @@ function VersionHoverCard({
10698
)
10799
.slice(0, 3);
108100

109-
return {
110-
header: <VersionHoverHeader releaseVersion={releaseVersion} />,
111-
body: (
112-
<Flex direction="column" gap={space(2)}>
113-
<Flex gap={space(2)} justify="space-between">
114-
<div>
115-
<h6>{t('New Issues')}</h6>
116-
<CountSince>{release.newGroups}</CountSince>
117-
</div>
118-
<div>
119-
<h6 style={{textAlign: 'right'}}>{t('Date Created')}</h6>
120-
<DateTime date={release.dateCreated} seconds={false} />
121-
</div>
122-
</Flex>
123-
{parsedVersion?.package && (
124-
<Flex direction="column" gap={space(2)} justify="space-between">
125-
{parsedVersion.package && (
126-
<div>
127-
<h6>{t('Package')}</h6>
128-
<div>{parsedVersion.package}</div>
129-
</div>
130-
)}
131-
{release.commitCount > 0 ? (
132-
<div>
133-
<h6>
134-
{release.commitCount}{' '}
135-
{release.commitCount === 1 ? t('commit ') : t('commits ')} {t('by ')}{' '}
136-
{release.authors.length}{' '}
137-
{release.authors.length === 1 ? t('author') : t('authors')}{' '}
138-
</h6>
139-
<AvatarListContainer>
140-
<AvatarList
141-
users={authors}
142-
avatarSize={25}
143-
tooltipOptions={{container: 'body'} as any}
144-
typeAvatars="authors"
145-
/>
146-
</AvatarListContainer>
147-
</div>
148-
) : null}
149-
</Flex>
150-
)}
151-
{release.lastCommit && <LastCommit commit={release.lastCommit} />}
152-
{deploys.length > 0 && (
153-
<Flex direction="column" gap={space(0.5)}>
154-
<h6>{t('Deploys')}</h6>
155-
{recentDeploysByEnvironment.map(deploy => {
156-
return (
157-
<Flex
158-
key={deploy.id}
159-
align="center"
160-
gap={space(1)}
161-
justify="space-between"
162-
>
163-
<Tag type="highlight">{deploy.environment}</Tag>
164-
{deploy.dateFinished && (
165-
<StyledTimeSince date={deploy.dateFinished} />
166-
)}
167-
</Flex>
168-
);
169-
})}
170-
</Flex>
171-
)}
101+
return (
102+
<Flex direction="column" gap={space(2)}>
103+
<Flex gap={space(2)} justify="space-between">
104+
<div>
105+
<h6>{t('New Issues')}</h6>
106+
<CountSince>{release.newGroups}</CountSince>
107+
</div>
108+
<div>
109+
<h6 style={{textAlign: 'right'}}>{t('Date Created')}</h6>
110+
<DateTime date={release.dateCreated} seconds={false} />
111+
</div>
172112
</Flex>
173-
),
174-
};
113+
{parsedVersion?.package && (
114+
<Flex direction="column" gap={space(2)} justify="space-between">
115+
{parsedVersion.package && (
116+
<div>
117+
<h6>{t('Package')}</h6>
118+
<div>{parsedVersion.package}</div>
119+
</div>
120+
)}
121+
{release.commitCount > 0 ? (
122+
<div>
123+
<h6>
124+
{release.commitCount}{' '}
125+
{release.commitCount === 1 ? t('commit ') : t('commits ')} {t('by ')}{' '}
126+
{release.authors.length}{' '}
127+
{release.authors.length === 1 ? t('author') : t('authors')}{' '}
128+
</h6>
129+
<AvatarListContainer>
130+
<AvatarList
131+
users={authors}
132+
avatarSize={25}
133+
tooltipOptions={{container: 'body'} as any}
134+
typeAvatars="authors"
135+
/>
136+
</AvatarListContainer>
137+
</div>
138+
) : null}
139+
</Flex>
140+
)}
141+
{release.lastCommit && <LastCommit commit={release.lastCommit} />}
142+
{deploys.length > 0 && (
143+
<Flex direction="column" gap={space(0.5)}>
144+
<h6>{t('Deploys')}</h6>
145+
{recentDeploysByEnvironment.map(deploy => {
146+
return (
147+
<Flex
148+
key={deploy.id}
149+
align="center"
150+
gap={space(1)}
151+
justify="space-between"
152+
>
153+
<Tag type="highlight">{deploy.environment}</Tag>
154+
{deploy.dateFinished && <StyledTimeSince date={deploy.dateFinished} />}
155+
</Flex>
156+
);
157+
})}
158+
</Flex>
159+
)}
160+
</Flex>
161+
);
175162
}
176163

177-
let header: React.ReactNode = null;
178-
let body: React.ReactNode = null;
179-
180-
const loading = !!(isDeploysLoading || isReleaseLoading || isRepositoriesLoading);
164+
const loading = isDeploysLoading || isReleaseLoading || isRepositoriesLoading;
181165
const error = isDeploysError ?? isReleaseError ?? isRepositoriesError;
182166
const hasRepos = repositories && repositories.length > 0;
183167

184168
if (loading) {
185-
body = <LoadingIndicator mini />;
186-
} else if (error) {
187-
body = <LoadingError />;
188-
} else {
189-
const renderObj: {body: React.ReactNode; header: React.ReactNode} =
190-
hasRepos && release ? getBody() : getRepoLink();
191-
header = renderObj.header;
192-
body = renderObj.body;
169+
return (
170+
<Flex justify="center">
171+
<LoadingIndicator mini />
172+
</Flex>
173+
);
193174
}
175+
if (error) {
176+
return <LoadingError />;
177+
}
178+
179+
return hasRepos && release ? getBody() : getRepoLink();
180+
}
181+
182+
interface Props extends React.ComponentProps<typeof Hovercard>, BodyProps {}
194183

195-
return <Hovercard {...hovercardProps} header={header} body={body} />;
184+
function VersionHoverCard({
185+
organization,
186+
projectSlug,
187+
releaseVersion,
188+
...hovercardProps
189+
}: Props) {
190+
return (
191+
<Hovercard
192+
{...hovercardProps}
193+
header={<VersionHoverHeader releaseVersion={releaseVersion} />}
194+
body={
195+
<VersionHoverCardBody
196+
organization={organization}
197+
projectSlug={projectSlug}
198+
releaseVersion={releaseVersion}
199+
/>
200+
}
201+
/>
202+
);
196203
}
197204

198205
interface VersionHoverHeaderProps {

0 commit comments

Comments
 (0)