Skip to content

Commit a03a54d

Browse files
buenos-nachosbuenos-nachos
authored
fix(site): resolve all Array.prototype.toSorted and Array.prototype.sort bugs (coder#17307)
Closes coder#16759 ## Changes made - Replaced all instances of `Array.prototype.toSorted` with `Array.prototype.sort` to provide better support for older browsers - Updated all `Array.prototype.sort` calls where necessary to remove risks of mutation render bugs - Refactored some code (moved things around, added comments) to make it more clear that certain `.sort` calls are harmless and don't have any risks
1 parent d17bcc7 commit a03a54d

File tree

9 files changed

+85
-86
lines changed

9 files changed

+85
-86
lines changed

‎site/src/api/queries/organizations.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ export const organizationsPermissions = (
270270
}
271271

272272
return {
273-
queryKey: ["organizations", organizationIds.sort(), "permissions"],
273+
queryKey: ["organizations", [...organizationIds.sort()], "permissions"],
274274
queryFn: async () => {
275275
// Only request what we need for the sidebar, which is one edit permission
276276
// per sub-link (settings, groups, roles, and members pages) that tells us
@@ -316,7 +316,7 @@ export const workspacePermissionsByOrganization = (
316316
}
317317

318318
return {
319-
queryKey: ["workspaces", organizationIds.sort(), "permissions"],
319+
queryKey: ["workspaces", [...organizationIds.sort()], "permissions"],
320320
queryFn: async () => {
321321
const prefixedChecks = organizationIds.flatMap((orgId) =>
322322
Object.entries(workspacePermissionChecks(orgId, userId)).map(

‎site/src/modules/dashboard/Navbar/proxyUtils.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export function sortProxiesByLatency(
44
proxies: Proxies,
55
latencies: ProxyLatencies,
66
) {
7-
return proxies.toSorted((a, b) => {
7+
return [...proxies].sort((a, b) => {
88
const latencyA = latencies?.[a.id]?.latencyMS ?? Number.POSITIVE_INFINITY;
99
const latencyB = latencies?.[b.id]?.latencyMS ?? Number.POSITIVE_INFINITY;
1010
return latencyA - latencyB;

‎site/src/modules/workspaces/WorkspaceTiming/Chart/utils.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ export const mergeTimeRanges = (ranges: TimeRange[]): TimeRange => {
1313
.sort((a, b) => a.startedAt.getTime() - b.startedAt.getTime());
1414
const start = sortedDurations[0].startedAt;
1515

16-
const sortedEndDurations = ranges
17-
.slice()
18-
.sort((a, b) => a.endedAt.getTime() - b.endedAt.getTime());
16+
const sortedEndDurations = [...ranges].sort(
17+
(a, b) => a.endedAt.getTime() - b.endedAt.getTime(),
18+
);
1919
const end = sortedEndDurations[sortedEndDurations.length - 1].endedAt;
2020
return { startedAt: start, endedAt: end };
2121
};

‎site/src/pages/CreateTemplateGalleryPage/StarterTemplates.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const sortVisibleTemplates = (templates: TemplateExample[]) => {
2626
// The docker template should be the first template in the list,
2727
// as it's the easiest way to get started with Coder.
2828
const dockerTemplateId = "docker";
29-
return templates.sort((a, b) => {
29+
return [...templates].sort((a, b) => {
3030
if (a.id === dockerTemplateId) {
3131
return -1;
3232
}

‎site/src/pages/DeploymentSettingsPage/LicensesSettingsPage/LicensesSettingsPageView.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ const LicensesSettingsPageView: FC<Props> = ({
9999

100100
{!isLoading && licenses && licenses?.length > 0 && (
101101
<Stack spacing={4} className="licenses">
102-
{licenses
102+
{[...(licenses ?? [])]
103103
?.sort(
104104
(a, b) =>
105105
new Date(b.claims.license_expires).valueOf() -

‎site/src/pages/HealthPage/DERPPage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export const DERPPage: FC = () => {
9191
<section>
9292
<SectionLabel>Regions</SectionLabel>
9393
<div css={{ display: "flex", flexWrap: "wrap", gap: 12 }}>
94-
{Object.values(regions!)
94+
{Object.values(regions ?? {})
9595
.filter((region) => {
9696
// Values can technically be null
9797
return region !== null;

‎site/src/pages/OrganizationSettingsPage/CustomRolesPage/CustomRolesPageView.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,8 @@ const RoleTable: FC<RoleTableProps> = ({
170170
</Cond>
171171

172172
<Cond>
173-
{roles
174-
?.sort((a, b) => a.name.localeCompare(b.name))
173+
{[...(roles ?? [])]
174+
.sort((a, b) => a.name.localeCompare(b.name))
175175
.map((role) => (
176176
<RoleRow
177177
key={role.name}

‎site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx‎

Lines changed: 72 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,9 @@ const TemplateUsagePanel: FC<TemplateUsagePanelProps> = ({
412412
...panelProps
413413
}) => {
414414
const theme = useTheme();
415-
const validUsage = data?.filter((u) => u.seconds > 0);
415+
const validUsage = data
416+
?.filter((u) => u.seconds > 0)
417+
.sort((a, b) => b.seconds - a.seconds);
416418
const totalInSeconds =
417419
validUsage?.reduce((total, usage) => total + usage.seconds, 0) ?? 1;
418420
const usageColors = chroma
@@ -438,86 +440,82 @@ const TemplateUsagePanel: FC<TemplateUsagePanelProps> = ({
438440
gap: 24,
439441
}}
440442
>
441-
{validUsage
442-
.sort((a, b) => b.seconds - a.seconds)
443-
.map((usage, i) => {
444-
const percentage = (usage.seconds / totalInSeconds) * 100;
445-
return (
446-
<div
447-
key={usage.slug}
448-
css={{ display: "flex", gap: 24, alignItems: "center" }}
449-
>
443+
{validUsage.map((usage, i) => {
444+
const percentage = (usage.seconds / totalInSeconds) * 100;
445+
return (
446+
<div
447+
key={usage.slug}
448+
css={{ display: "flex", gap: 24, alignItems: "center" }}
449+
>
450+
<div css={{ display: "flex", alignItems: "center", gap: 8 }}>
450451
<div
451-
css={{ display: "flex", alignItems: "center", gap: 8 }}
452-
>
453-
<div
454-
css={{
455-
width: 20,
456-
height: 20,
457-
display: "flex",
458-
alignItems: "center",
459-
justifyContent: "center",
460-
}}
461-
>
462-
<img
463-
src={usage.icon}
464-
alt=""
465-
style={{
466-
objectFit: "contain",
467-
width: "100%",
468-
height: "100%",
469-
}}
470-
/>
471-
</div>
472-
<div css={{ fontSize: 13, fontWeight: 500, width: 200 }}>
473-
{usage.display_name}
474-
</div>
475-
</div>
476-
<Tooltip
477-
title={`${Math.floor(percentage)}%`}
478-
placement="top"
479-
arrow
452+
css={{
453+
width: 20,
454+
height: 20,
455+
display: "flex",
456+
alignItems: "center",
457+
justifyContent: "center",
458+
}}
480459
>
481-
<LinearProgress
482-
value={percentage}
483-
variant="determinate"
484-
css={{
460+
<img
461+
src={usage.icon}
462+
alt=""
463+
style={{
464+
objectFit: "contain",
485465
width: "100%",
486-
height: 8,
487-
backgroundColor: theme.palette.divider,
488-
"& .MuiLinearProgress-bar": {
489-
backgroundColor: usageColors[i],
490-
borderRadius: 999,
491-
},
466+
height: "100%",
492467
}}
493468
/>
494-
</Tooltip>
495-
<Stack
496-
spacing={0}
469+
</div>
470+
<div css={{ fontSize: 13, fontWeight: 500, width: 200 }}>
471+
{usage.display_name}
472+
</div>
473+
</div>
474+
<Tooltip
475+
title={`${Math.floor(percentage)}%`}
476+
placement="top"
477+
arrow
478+
>
479+
<LinearProgress
480+
value={percentage}
481+
variant="determinate"
497482
css={{
498-
fontSize: 13,
499-
color: theme.palette.text.secondary,
500-
width: 120,
501-
flexShrink: 0,
502-
lineHeight: "1.5",
483+
width: "100%",
484+
height: 8,
485+
backgroundColor: theme.palette.divider,
486+
"& .MuiLinearProgress-bar": {
487+
backgroundColor: usageColors[i],
488+
borderRadius: 999,
489+
},
503490
}}
504-
>
505-
{formatTime(usage.seconds)}
506-
{usage.times_used > 0 && (
507-
<span
508-
css={{
509-
fontSize: 12,
510-
color: theme.palette.text.disabled,
511-
}}
512-
>
513-
Opened {usage.times_used.toLocaleString()}{" "}
514-
{usage.times_used === 1 ? "time" : "times"}
515-
</span>
516-
)}
517-
</Stack>
518-
</div>
519-
);
520-
})}
491+
/>
492+
</Tooltip>
493+
<Stack
494+
spacing={0}
495+
css={{
496+
fontSize: 13,
497+
color: theme.palette.text.secondary,
498+
width: 120,
499+
flexShrink: 0,
500+
lineHeight: "1.5",
501+
}}
502+
>
503+
{formatTime(usage.seconds)}
504+
{usage.times_used > 0 && (
505+
<span
506+
css={{
507+
fontSize: 12,
508+
color: theme.palette.text.disabled,
509+
}}
510+
>
511+
Opened {usage.times_used.toLocaleString()}{" "}
512+
{usage.times_used === 1 ? "time" : "times"}
513+
</span>
514+
)}
515+
</Stack>
516+
</div>
517+
);
518+
})}
521519
</div>
522520
)}
523521
</PanelContent>

‎site/src/pages/WorkspacePage/AppStatuses.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ export const AppStatuses: FC<AppStatusesProps> = ({
165165
})),
166166
);
167167

168-
// 2. Sort statuses chronologically (newest first)
168+
// 2. Sort statuses chronologically (newest first) - mutating the value is
169+
// fine since it's not an outside parameter
169170
allStatuses.sort(
170171
(a, b) =>
171172
new Date(b.created_at).getTime() - new Date(a.created_at).getTime(),

0 commit comments

Comments
 (0)