Skip to content

Commit 8bedc58

Browse files
committed
fix: keep download selections when the version changes route
Picking a version whose status belongs to another route (/download, /download/current, /download/alpha) called redirect(). All three routes are served by the same [...path] page, so Next remounted the page and the release providers restarted from their defaults. That reset the architecture to x64 (or the detected one), and also the OS, install method, package manager and the picked version, which fell back to the newest release of the destination route. The version now changes in place, like it already did within a route, and the URL is updated with history.replaceState, which Next syncs with usePathname. The redirect on load that normalizes the initial route stays. Assisted-by: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent 53d777b commit 8bedc58

4 files changed

Lines changed: 152 additions & 32 deletions

File tree

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import WithNoScriptSelect from '@node-core/ui-components/Common/Select/NoScriptS
44
import { useLocale, useTranslations } from 'next-intl';
55
import { use, useEffect } from 'react';
66

7-
import { redirect, usePathname } from '#site/navigation.mjs';
8-
import { STATUS_KIND_MAP } from '#site/next.constants.mjs';
7+
import { getPathname, redirect, usePathname } from '#site/navigation.mjs';
8+
import { BASE_PATH, STATUS_KIND_MAP } from '#site/next.constants.mjs';
99
import {
1010
ReleaseContext,
1111
ReleasesContext,
@@ -46,21 +46,23 @@ const VersionDropdown: FC = () => {
4646
// eslint-disable-next-line @eslint-react/exhaustive-deps -- deps are correct on first render, and this should not run again.
4747
}, []);
4848

49-
// Handle setting the new version or navigating to the correct route based on status.
50-
const setVersionOrNavigate = (version: string) => {
49+
// Sets the new version and keeps the URL matching its status. The URL is replaced
50+
// in place, as navigating remounts the page and discards the user's selections.
51+
const setVersionAndSyncRoute = (version: string) => {
5152
const selectedRelease = releases.find(
5253
({ versionWithPrefix }) => versionWithPrefix === version
5354
);
5455

55-
if (selectedRelease) {
56-
const href = getRedirectPath(selectedRelease.status);
57-
if (href) {
58-
redirect({ href, locale });
59-
return;
60-
}
61-
}
62-
6356
setVersion(version);
57+
58+
const href = selectedRelease && getRedirectPath(selectedRelease.status);
59+
if (href) {
60+
window.history.replaceState(
61+
null,
62+
'',
63+
`${BASE_PATH}${getPathname({ href, locale })}`
64+
);
65+
}
6466
};
6567

6668
return (
@@ -72,7 +74,7 @@ const VersionDropdown: FC = () => {
7274
badge: { label: status, kind: STATUS_KIND_MAP[status] },
7375
}))}
7476
defaultValue={release.versionWithPrefix}
75-
onChange={setVersionOrNavigate}
77+
onChange={setVersionAndSyncRoute}
7678
className="min-w-36"
7779
inline={true}
7880
/>
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import assert from 'node:assert/strict';
2+
import { beforeEach, describe, it, mock } from 'node:test';
3+
4+
import { act, render } from '@testing-library/react';
5+
6+
import { MatterContext } from '#site/providers/matterProvider';
7+
import {
8+
ReleaseProvider,
9+
ReleasesProvider,
10+
} from '#site/providers/releaseProvider';
11+
12+
const VERSION = 'layouts.download.dropdown.version';
13+
const PLATFORM = 'layouts.download.dropdown.platform';
14+
15+
const selects = {};
16+
const redirect = mock.fn();
17+
let pathname;
18+
19+
const replaceState = mock.method(window.history, 'replaceState', (...args) => {
20+
pathname = args[2].replace('/en', '');
21+
});
22+
23+
const Select = props => {
24+
selects[props.ariaLabel] = props;
25+
return null;
26+
};
27+
28+
mock.module('@node-core/ui-components/Common/Select', {
29+
defaultExport: Select,
30+
});
31+
32+
mock.module('@node-core/ui-components/Common/Select/NoScriptSelect', {
33+
defaultExport: Select,
34+
});
35+
36+
mock.module('#site/navigation.mjs', {
37+
namedExports: {
38+
redirect,
39+
getPathname: ({ href, locale }) => `/${locale}${href}`,
40+
usePathname: () => pathname,
41+
},
42+
});
43+
44+
const { default: OperatingSystemDropdown } =
45+
await import('../OperatingSystemDropdown');
46+
const { default: PlatformDropdown } = await import('../PlatformDropdown');
47+
const { default: VersionDropdown } = await import('../VersionDropdown');
48+
49+
const releases = [
50+
{ versionWithPrefix: 'v26.0.0-alpha.1', status: 'Alpha' },
51+
{ versionWithPrefix: 'v25.0.0', status: 'Current' },
52+
{ versionWithPrefix: 'v24.0.0', status: 'LTS' },
53+
{ versionWithPrefix: 'v18.0.0', status: 'EOL' },
54+
];
55+
56+
const renderDownloadPage = () =>
57+
render(
58+
<MatterContext value={{ os: 'LINUX', architecture: 'x64', bitness: '64' }}>
59+
<ReleasesProvider releases={releases} snippets={[]}>
60+
<ReleaseProvider initialRelease={releases[2]}>
61+
<VersionDropdown />
62+
<ReleaseProvider>
63+
<OperatingSystemDropdown />
64+
<PlatformDropdown />
65+
</ReleaseProvider>
66+
</ReleaseProvider>
67+
</ReleasesProvider>
68+
</MatterContext>
69+
);
70+
71+
describe('PlatformDropdown', () => {
72+
beforeEach(() => {
73+
pathname = '/download';
74+
redirect.mock.resetCalls();
75+
replaceState.mock.resetCalls();
76+
});
77+
78+
it('defaults to the detected platform', () => {
79+
renderDownloadPage();
80+
81+
assert.equal(selects[PLATFORM].value, 'x64');
82+
});
83+
84+
it('keeps the selected platform when the version changes route', () => {
85+
renderDownloadPage();
86+
87+
act(() => selects[PLATFORM].onChange('arm64'));
88+
assert.equal(selects[PLATFORM].value, 'arm64');
89+
90+
for (const { versionWithPrefix } of [
91+
releases[0],
92+
releases[1],
93+
releases[3],
94+
]) {
95+
act(() => selects[VERSION].onChange(versionWithPrefix));
96+
97+
assert.equal(selects[VERSION].defaultValue, versionWithPrefix);
98+
assert.equal(selects[PLATFORM].value, 'arm64');
99+
}
100+
101+
assert.equal(redirect.mock.callCount(), 0);
102+
assert.deepEqual(
103+
replaceState.mock.calls.map(call => call.arguments[2]),
104+
['/en/download/alpha', '/en/download/current', '/en/download']
105+
);
106+
});
107+
108+
it('keeps the selected platform when the version stays on the route', () => {
109+
renderDownloadPage();
110+
111+
act(() => selects[PLATFORM].onChange('arm64'));
112+
act(() => selects[VERSION].onChange('v18.0.0'));
113+
114+
assert.equal(selects[VERSION].defaultValue, 'v18.0.0');
115+
assert.equal(selects[PLATFORM].value, 'arm64');
116+
assert.equal(replaceState.mock.callCount(), 0);
117+
});
118+
});

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

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111

1212
const redirect = mock.fn();
1313
const setVersion = mock.fn();
14+
const replaceState = mock.method(window.history, 'replaceState', () => {});
1415
const Select = mock.fn(() => null);
1516
let pathname;
1617

@@ -21,6 +22,7 @@ mock.module('@node-core/ui-components/Common/Select/NoScriptSelect', {
2122
mock.module('#site/navigation.mjs', {
2223
namedExports: {
2324
redirect,
25+
getPathname: ({ href, locale }) => `/${locale}${href}`,
2426
usePathname: () => pathname,
2527
},
2628
});
@@ -90,6 +92,7 @@ describe('VersionDropdown', () => {
9092
beforeEach(() => {
9193
pathname = '/download';
9294
redirect.mock.resetCalls();
95+
replaceState.mock.resetCalls();
9396
setVersion.mock.resetCalls();
9497
Select.mock.resetCalls();
9598
});
@@ -156,25 +159,21 @@ describe('VersionDropdown', () => {
156159

157160
act(() => onChange(version));
158161

162+
assert.equal(redirect.mock.callCount(), 0);
163+
assert.equal(setVersion.mock.callCount(), 1);
164+
assert.deepEqual(setVersion.mock.calls[0].arguments, [version]);
165+
assert.equal(
166+
Select.mock.calls.at(-1).arguments[0].defaultValue,
167+
version
168+
);
169+
159170
if (target.path !== path) {
160-
assert.equal(redirect.mock.callCount(), 1);
161-
assert.deepEqual(redirect.mock.calls[0].arguments, [
162-
{ href: target.path, locale: 'en' },
163-
]);
164-
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
171+
assert.deepEqual(
172+
replaceState.mock.calls.map(call => call.arguments),
173+
[[null, '', `/en${target.path}`]]
169174
);
170175
} else {
171-
assert.equal(redirect.mock.callCount(), 0);
172-
assert.equal(setVersion.mock.callCount(), 1);
173-
assert.deepEqual(setVersion.mock.calls[0].arguments, [version]);
174-
assert.equal(
175-
Select.mock.calls.at(-1).arguments[0].defaultValue,
176-
version
177-
);
176+
assert.equal(replaceState.mock.callCount(), 0);
178177
}
179178
});
180179
}

‎apps/site/navigation.mjs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { availableLocaleCodes } from '@node-core/website-i18n';
44
import { createNavigation } from 'next-intl/navigation';
55

6-
export const { Link, redirect, usePathname, useRouter } = createNavigation({
7-
locales: availableLocaleCodes,
8-
});
6+
export const { Link, redirect, getPathname, usePathname, useRouter } =
7+
createNavigation({
8+
locales: availableLocaleCodes,
9+
});

0 commit comments

Comments
 (0)