fix: hide irrelevant download install options#8799
fix: hide irrelevant download install options#8799abhijeetnardele24-hash wants to merge 1 commit intonodejs:mainfrom
Conversation
Signed-off-by: abhijeet nardele <234410808+abhijeetnardele24-hash@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Also marks Reviewed by Cursor Bugbot for commit a4a03e7. Bugbot is set up for automated code reviews on this repo. Configure here. |
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
There was a problem hiding this comment.
Pull request overview
This PR updates the download page installation-method UX so users only see install options that are actually available for the currently selected OS, and aligns nvm compatibility with AIX (per #8638), adding a regression test to prevent future regressions.
Changes:
- Add AIX to
nvm’s supported OS list in download constants. - Filter out disabled (incompatible) install methods from the installation method dropdown, and remove empty option groups.
- Add a regression test ensuring
nvmremains enabled for AIX.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/site/util/download/constants.json | Marks nvm as compatible with AIX so it’s offered as an install method there. |
| apps/site/components/Downloads/Release/InstallationMethodDropdown.tsx | Hides incompatible install methods (instead of showing disabled entries) and drops empty groups. |
| apps/site/util/tests/download.test.mjs | Adds regression coverage for AIX + nvm compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // We group Platforms on the Platform Dropdown to provide the User | ||
| // understanding of what is recommended/official and what is not. | ||
| const grouppedMethods = useMemo( |
There was a problem hiding this comment.
The comments in this component still refer to “Platforms” / “Platform Dropdown”, but this is the installation method dropdown. Please update the wording to avoid confusion when maintaining this logic (e.g., grouping install methods into official/unofficial).
| // Sets either the utmost recommended platform or the first non-disabled one | ||
| // Note that the first item of groupped platforms is always the recommended one |
There was a problem hiding this comment.
After filtering out empty groups, grouppedMethods[0] isn’t guaranteed to be the recommended/official group anymore (it will be unofficial when there are no recommended methods). The inline comment about the first group always being recommended should be updated to match the new behavior to avoid future confusion.
| // Sets either the utmost recommended platform or the first non-disabled one | |
| // Note that the first item of groupped platforms is always the recommended one | |
| // Sets the first available item from the first non-empty group | |
| // (recommended/official when present, otherwise unofficial), or falls back | |
| // to the first non-disabled installation method. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4a03e7. Configure here.
| release.setInstallMethod(installationMethod as InstallationMethod); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [parsedInstallMethods, release.installMethod, release.os]); |
There was a problem hiding this comment.
Unguarded array access after filtering may crash
Medium Severity
grouppedMethods[0].items is accessed without a guard, but grouppedMethods can now be empty after the newly added .filter(({ items }) => items.length > 0). Previously the array always had exactly two group objects (official and unofficial), making index-zero access safe. Now, if all install methods are disabled for a given OS (e.g., due to future changes to constants.json), grouppedMethods[0] is undefined and accessing .items throws a TypeError at runtime.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a4a03e7. Configure here.
Summary
nvmas compatible with AIX, matching the maintainer confirmation in Remove irrelevant dropdown options for installing Node.js rather than disabling them #8638nvmcompatibility coveredFixes #8638.
Validation
node --experimental-test-coverage --test-coverage-exclude=**/*.test.* --experimental-test-module-mocks --enable-source-maps --import=global-jsdom/register --import=tsx --import=tests/setup.jsx --test util/__tests__/download.test.mjscorepack pnpm exec prettier --check apps/site/components/Downloads/Release/InstallationMethodDropdown.tsx apps/site/util/download/constants.json apps/site/util/__tests__/download.test.mjsNotes
pnpmbinary in this environment, so the commit was created with--no-verifyafter running the focused checks above manually.