Skip to content

Commit bc4348d

Browse files
authored
fix: ensure initial download route is normalized (#9247)
* Ensure initial download route is normalized * Wrap in useEffect to run once * Simplify logic
1 parent c57a8ab commit bc4348d

2 files changed

Lines changed: 82 additions & 22 deletions

File tree

‎apps/site/components/Downloads/Release/VersionDropdown.tsx‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import WithNoScriptSelect from '@node-core/ui-components/Common/Select/NoScriptSelect';
44
import { useLocale, useTranslations } from 'next-intl';
5-
import { use } from 'react';
5+
import { use, useEffect } from 'react';
66

77
import { redirect, usePathname } from '#site/navigation.mjs';
88
import { STATUS_KIND_MAP } from '#site/next.constants.mjs';
@@ -11,6 +11,7 @@ import {
1111
ReleasesContext,
1212
} from '#site/providers/releaseProvider';
1313

14+
import type { NodeReleaseStatus } from '#site/types';
1415
import type { FC } from 'react';
1516

1617
const VersionDropdown: FC = () => {
@@ -22,27 +23,41 @@ const VersionDropdown: FC = () => {
2223

2324
// Allows us to keep the route semantically correct to what the user should expect
2425
// from the /current and non /current routes.
25-
const setVersionOrNavigate = (version: string) => {
26-
const release = releases.find(
27-
({ versionWithPrefix }) => versionWithPrefix === version
28-
);
26+
const getRedirectPath = (status: NodeReleaseStatus) => {
27+
if (['LTS', 'EOL'].includes(status) && !pathname.endsWith('/download')) {
28+
return '/download';
29+
}
2930

30-
if (
31-
(release?.status === 'LTS' || release?.status === 'EOL') &&
32-
(pathname.includes('current') || pathname.includes('alpha'))
33-
) {
34-
redirect({ href: '/download', locale });
35-
return;
31+
if (status === 'Current' && !pathname.includes('current')) {
32+
return '/download/current';
3633
}
3734

38-
if (release?.status === 'Current' && !pathname.includes('current')) {
39-
redirect({ href: '/download/current', locale });
40-
return;
35+
if (status === 'Alpha' && !pathname.includes('alpha')) {
36+
return '/download/alpha';
4137
}
38+
};
39+
40+
// Ensure the initial route is correct based on the release status.
41+
useEffect(() => {
42+
const redirectPath = getRedirectPath(release.status);
43+
if (redirectPath) {
44+
redirect({ href: redirectPath, locale });
45+
}
46+
// eslint-disable-next-line @eslint-react/exhaustive-deps -- deps are correct on first render, and this should not run again.
47+
}, []);
48+
49+
// Handle setting the new version or navigating to the correct route based on status.
50+
const setVersionOrNavigate = (version: string) => {
51+
const selectedRelease = releases.find(
52+
({ versionWithPrefix }) => versionWithPrefix === version
53+
);
4254

43-
if (release?.status === 'Alpha' && !pathname.includes('alpha')) {
44-
redirect({ href: '/download/alpha', locale });
45-
return;
55+
if (selectedRelease) {
56+
const href = getRedirectPath(selectedRelease.status);
57+
if (href) {
58+
redirect({ href, locale });
59+
return;
60+
}
4661
}
4762

4863
setVersion(version);

‎apps/site/components/Downloads/Release/__tests__/VersionDropdown.test.jsx‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import assert from 'node:assert/strict';
22
import { beforeEach, describe, it, mock } from 'node:test';
33

4-
import { render } from '@testing-library/react';
4+
import { act, render } from '@testing-library/react';
5+
import { useState } from 'react';
56

67
import {
78
ReleaseContext,
@@ -58,11 +59,27 @@ const releases = statuses.flatMap(({ status, versions }) =>
5859
);
5960

6061
const renderDropdown = (release = releases[0]) => {
61-
render(
62-
<ReleasesContext value={{ releases, snippets: [] }}>
63-
<ReleaseContext value={{ release, setVersion }}>
62+
const Dropdown = () => {
63+
const [version, updateVersion] = useState(release.versionWithPrefix);
64+
setVersion.mock.mockImplementation(updateVersion);
65+
66+
return (
67+
<ReleaseContext
68+
value={{
69+
release: releases.find(
70+
candidate => candidate.versionWithPrefix === version
71+
),
72+
setVersion,
73+
}}
74+
>
6475
<VersionDropdown />
6576
</ReleaseContext>
77+
);
78+
};
79+
80+
render(
81+
<ReleasesContext value={{ releases, snippets: [] }}>
82+
<Dropdown />
6683
</ReleasesContext>
6784
);
6885

@@ -90,6 +107,7 @@ describe('VersionDropdown', () => {
90107

91108
assert.equal(defaultValue, release.versionWithPrefix);
92109
assert.equal(ariaLabel, 'layouts.download.dropdown.version');
110+
assert.equal(redirect.mock.callCount(), 0);
93111
});
94112

95113
it(`provides the ${status} badge for each version`, () => {
@@ -109,27 +127,54 @@ describe('VersionDropdown', () => {
109127
});
110128

111129
for (const target of statuses) {
130+
it(`normalizes ${target.path} on load for ${status}`, () => {
131+
pathname = target.path;
132+
133+
renderDropdown(release);
134+
135+
if (target.path !== path) {
136+
assert.deepEqual(
137+
redirect.mock.calls.map(call => call.arguments),
138+
[[{ href: path, locale: 'en' }]]
139+
);
140+
} else {
141+
assert.equal(redirect.mock.callCount(), 0);
142+
}
143+
assert.equal(setVersion.mock.callCount(), 0);
144+
});
145+
112146
const description =
113147
target.status === status ? `another ${status}` : target.status;
114148

115149
it(`switches to ${description}`, () => {
116150
const { onChange } = renderDropdown(release);
151+
redirect.mock.resetCalls();
152+
setVersion.mock.resetCalls();
117153
const version = target.versions.find(
118154
candidate => candidate !== release.versionWithPrefix
119155
);
120156

121-
onChange(version);
157+
act(() => onChange(version));
122158

123159
if (target.path !== path) {
124160
assert.equal(redirect.mock.callCount(), 1);
125161
assert.deepEqual(redirect.mock.calls[0].arguments, [
126162
{ href: target.path, locale: 'en' },
127163
]);
128164
assert.equal(setVersion.mock.callCount(), 0);
165+
assert.equal(Select.mock.callCount(), 1);
166+
assert.equal(
167+
Select.mock.calls.at(-1).arguments[0].defaultValue,
168+
release.versionWithPrefix
169+
);
129170
} else {
130171
assert.equal(redirect.mock.callCount(), 0);
131172
assert.equal(setVersion.mock.callCount(), 1);
132173
assert.deepEqual(setVersion.mock.calls[0].arguments, [version]);
174+
assert.equal(
175+
Select.mock.calls.at(-1).arguments[0].defaultValue,
176+
version
177+
);
133178
}
134179
});
135180
}

0 commit comments

Comments
 (0)