Skip to content

feat(ui): Reduce the number of re-fetches on release details #29507

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
matejminar merged 2 commits into from
Oct 25, 2021
Merged

feat(ui): Reduce the number of re-fetches on release details #29507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e833890
feat(ui): Reduce the number of re-fetches on release details
matejminar Oct 22, 2021
6c5611b
convert array to string in useEffect dependencies
matejminar Oct 25, 2021
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
37 changes: 34 additions & 3 deletions static/app/views/releases/detail/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {createContext} from 'react';
import {RouteComponentProps} from 'react-router';
import styled from '@emotion/styled';
import {Location} from 'history';
import isEqual from 'lodash/isEqual';
import pick from 'lodash/pick';

import Alert from 'app/components/alert';
Expand All @@ -10,7 +12,7 @@ import NoProjectMessage from 'app/components/noProjectMessage';
import GlobalSelectionHeader from 'app/components/organizations/globalSelectionHeader';
import {getParams} from 'app/components/organizations/globalSelectionHeader/getParams';
import PickProjectToContinue from 'app/components/pickProjectToContinue';
import {URL_PARAM} from 'app/constants/globalSelectionHeader';
import {PAGE_URL_PARAM, URL_PARAM} from 'app/constants/globalSelectionHeader';
import {IconInfo, IconWarning} from 'app/icons';
import {t} from 'app/locale';
import {PageContent} from 'app/styles/organization';
Expand Down Expand Up @@ -90,6 +92,21 @@ class ReleasesDetail extends AsyncView<Props, State> {
};
}

componentDidUpdate(prevProps, prevContext: Record<string, any>) {
Copy link
Member

@priscilawebdev priscilawebdev Oct 25, 2021

Choose a reason for hiding this comment

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

wouldn't the second parameter be "prevState"?

Copy link
Member Author

@matejminar matejminar Oct 25, 2021

Choose a reason for hiding this comment

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

yeah, I think the confusion comes from this line:

sentry/static/app/components/asyncComponent.tsx

Line 120 in 652843d

componentDidUpdate(prevProps: P, prevContext: any) {

We have it named like this in a couple of different places, can open a follow-up PR to rename it everywhere.

const {organization, params, location} = this.props;

if (
prevProps.params.release !== params.release ||
prevProps.organization.slug !== organization.slug ||
!isEqual(
this.pickLocationQuery(prevProps.location),
this.pickLocationQuery(location)
)
) {
super.componentDidUpdate(prevProps, prevContext);
}
}

getEndpoints(): ReturnType<AsyncComponent['getEndpoints']> {
const {organization, location, params, releaseMeta} = this.props;

Expand All @@ -104,7 +121,7 @@ class ReleasesDetail extends AsyncView<Props, State> {
{
query: {
adoptionStages: 1,
...getParams(pick(location.query, [...Object.values(URL_PARAM)])),
...getParams(this.pickLocationQuery(location)),
},
},
],
Expand Down Expand Up @@ -133,6 +150,13 @@ class ReleasesDetail extends AsyncView<Props, State> {
return endpoints;
}

pickLocationQuery(location: Location) {
return pick(location.query, [
...Object.values(URL_PARAM),
...Object.values(PAGE_URL_PARAM),
]);
}

renderError(...args) {
const possiblyWrongProject = Object.values(this.state.errors).find(
e => e?.status === 404 || e?.status === 403
Expand Down Expand Up @@ -227,8 +251,15 @@ class ReleasesDetailContainer extends AsyncComponent<
}

componentDidUpdate(prevProps, prevContext: Record<string, any>) {
super.componentDidUpdate(prevProps, prevContext);
const {organization, params} = this.props;

this.removeGlobalDateTimeFromUrl();
if (
prevProps.params.release !== params.release ||
prevProps.organization.slug !== organization.slug
) {
super.componentDidUpdate(prevProps, prevContext);
}
}

removeGlobalDateTimeFromUrl() {
Expand Down
10 changes: 9 additions & 1 deletion static/app/views/releases/detail/overview/releaseComparisonChart/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,15 @@ function ReleaseComparisonChart({
fetchEventsTotals();
fetchIssuesTotals();
}
}, [period, start, end, organization.slug, location]);
}, [
period,
start,
end,
organization.slug,
location.query.project,
location.query.environment?.toString(),
release.version,
]);

useEffect(() => {
const chartInUrl = decodeScalar(location.query.chart) as ReleaseComparisonChartType;
Expand Down