Skip to content

chore(wsl): add notice #7758

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.

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Vishal-K-988
Copy link
Contributor

@Vishal-K-988 Vishal-K-988 commented May 18, 2025

Description

Raised a new PR as previous PR adds unwanted files

This PR introduces a new feature to the download page:
When a user selects "Windows" as their operating system in any of the OS dropdowns, a WSL (Windows Subsystem for Linux) informational message is displayed directly below the "Download Node.js®" title.
The message provides users with helpful information about WSL and a link to learn more, improving the experience for Windows users interested in running Node.js in a Linux environment.

Key changes:

  • Added WSLMessage and WindowsWSLMessage components.
  • Registered these components in the MDX configuration for use in MDX files.
  • Refactored context usage to allow independent OS selection in each dropdown.
  • Ensured the WSL message appears only once, in a consistent and user-friendly location.

Validation

  • Manually tested the download page.
  • When "Windows" is selected in any OS dropdown, the WSL message appears below the main title.
  • The message disappears when another OS is selected.
  • Both OS dropdowns can now be used independently.
  • The UI remains clean and consistent.
  • Working Video
    Working video Drive Link

Related Issues

Addresses #7651

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

… utils

>>
>> Moved constants.json and index.tsx into a dedicated subdirectory.
>> Reorganized constants such as PLATFORMS into constants.json using object destructuring.
>> Similarly for OS_NOT_SUPPORTING_INSTALLERS, OperatingSystems
>>
>> Fixes: nodejs#7561

Signed-off-by: vishal <[email protected]>
Copy link

vercel bot commented May 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview May 20, 2025 4:45am

Copy link
Contributor

Note

Your Pull Request seems to be updating Translations of the Node.js Website.

Whilst we appreciate your intent; Any Translation update should be done through our Crowdin Project.
We recommend giving a read on our Translation Guidelines.

Thank you!

- Add WSLMessage component to display information about Windows Subsystem for Linux (WSL)
- Show WSLMessage only once, directly below the `Download Node.js®`  title, when Windows(WIN) is selected in any OS dropdown
- Refactor context usage to allow independent OS selection in each dropdown
- Register WSLMessage and WindowsWSLMessage in MDX components for use in MDX files
- Improve user experience by providing relevant WSL info at the right place and time
- Enhanced both for Dark and Light Theme with Blue background and border and glowing theme perfectly matching the ui theme of the website !
Issue nodejs#7651

Signed-off-by: vishal <[email protected]>
@Vishal-K-988 Vishal-K-988 marked this pull request as ready for review May 18, 2025 15:43
@Vishal-K-988 Vishal-K-988 requested a review from a team as a code owner May 18, 2025 15:43
@avivkeller avivkeller changed the title refactor(download): extract constants to JSON and reorganize download… chore(wsl): add noticd May 18, 2025
@Vishal-K-988 Vishal-K-988 changed the title chore(wsl): add noticd feat(WSL) : Adds a new WSL description in the download section May 18, 2025
@Vishal-K-988 Vishal-K-988 changed the title feat(WSL) : Adds a new WSL description in the download section chore(wsl): add noticd May 18, 2025
@avivkeller avivkeller changed the title chore(wsl): add noticd chore(wsl): add notice May 18, 2025
Comment on lines 285 to 290
},
"wsl": {
"title": "Windows를 사용 중이신가요? WSL을 사용해 보세요 🦖",
"description": "Windows Subsystem for Linux (WSL)을 사용하면 가상 머신 없이 Windows에서 직접 Linux 환경을 실행할 수 있습니다. 이는 Node.js 애플리케이션 개발 경험을 향상시킬 수 있습니다.",
"learnMore": "WSL에 대해 자세히 알아보기"
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you only change the English files? Crowdin will update the other languages eventually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing all the changes made to all the other languages!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recently saw the vercel deployment and when changed the language to Japanese, still shows the message in english
(not in translated to japanese )
url : https://nodejs-gdlsqs291-openjs.vercel.app/ja/download
image

Copy link
Member

Choose a reason for hiding this comment

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

That's okay. Crowdin handles translations

@@ -3,6 +3,8 @@ layout: download
title: Download Node.js®
---

<WindowsWSLMessage />
Copy link
Member

Choose a reason for hiding this comment

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

This can be part of the Layout, right?

And can't AlertBox be used instead of a new component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I want <WindowsWSLMessage /> component to appear on certain pages, so I added this to download/index.mdx , not to the layout. Adding it to the layout would make it appear on every page.

  2. At first, I considered using an alert box, but it creates a pop-up animation, which I think can be distracting and might negatively impact the user experience.
    On the other hand, this new component feels like a natural part of the website and looks more integrated.

Copy link
Member

Choose a reason for hiding this comment

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

AlertBox doesn't make a popup, I think you are thinking of Modal. If you select a community option for Download (rather than a Recommended), you'll see an alert box that says "Installation methods that involve community software are supported by the teams maintaining that software.", we can add one above for WSL.

It can say something like:

"These instructions are meant to be ran in a PowerShell terminal. If you are using [Windows Subsystem for Linux](link to ms docs), please refer to the Linux instructions"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but I couldn't find the "Community Installation" method / option at nodejs website!

can you please guide me through url or something so that I can get a rough idea about AlertBox

Copy link
Member

Choose a reason for hiding this comment

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

@Vishal-K-988 If you select 'Windows' and 'Chocolatey' you should see the AlertBox component (it's the blue info box).

@@ -3,6 +3,8 @@ layout: download
title: Descarga Node.js®
---

<WindowsWSLMessage />
Copy link
Member

Choose a reason for hiding this comment

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

Also these changes can be reverted, only the /en/ directory should be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crowdin will handle for rest of the supported languages, Right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & Committed !

@@ -3,6 +3,8 @@ layout: download
title: Download Node.js®
---

<WindowsWSLMessage />
Copy link
Member

Choose a reason for hiding this comment

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

AlertBox doesn't make a popup, I think you are thinking of Modal. If you select a community option for Download (rather than a Recommended), you'll see an alert box that says "Installation methods that involve community software are supported by the teams maintaining that software.", we can add one above for WSL.

It can say something like:

"These instructions are meant to be ran in a PowerShell terminal. If you are using [Windows Subsystem for Linux](link to ms docs), please refer to the Linux instructions"

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

I haven't reviewed the code, but I understand the idea. However, i think it would be good to add an item to the select with Windows (WSL)

@avivkeller
Copy link
Member

Wouldn't Linux (or WSL) better?

…ges other than english ` en ` , from ` apps/site/pages/` directory

Signed-off-by: vishal <[email protected]>
@Vishal-K-988
Copy link
Contributor Author

Wouldn't Linux (or WSL) better?

I think that adding a Windows (WSL) option would be helpful! The Node.js website should be easy for newcomers, but most new users aren’t familiar with WSL. If we provide a WSL option when “Windows” is selected (since there are more Windows users than Linux users), it could introduce people to WSL and make the process smoother.

As Sebastian mentioned, having a separate dropdown option for Windows (WSL) is also a good idea.

Let me know your thoughts so we can move forward!

@ovflowd
Copy link
Member

ovflowd commented May 19, 2025

Wouldn't Linux (or WSL) better?

Im strictly against on coupling WSL stuff with Linux. Also, WSL is not an OS it is a layer upon paravirtualization such as Hyper-V. Shouldn't be its own thing. I'm fine having entries for WSL on the upcoming page that @canerakdas is making for all possible options; But I'm -1 on making this a main option on the Dropdown. At the maximum, I'm fine having a link pointing to it, but feels far-fetched. If we start adding OS-specific solutions or vendors such as WSL, or Cygwin or what not.

@Vishal-K-988
Copy link
Contributor Author

As discussed earlier in this issue, I’m currently uncertain about how I should proceed and would appreciate further guidance on the next steps.

@ovflowd
Copy link
Member

ovflowd commented May 25, 2025

It is tricky, because you're "adding a huge banner" that is visible for any download option, regardless if you've selected macOS, Windows, or Linux for something that is super niche. WSL is good, I use it, but the same Linux installation instructions apply because you're simply just running a VM

I do believe folks running WSL are knowledged enough to realize that, and simply install a Linux option. We shouldn't have bindings for a specific toolchain for a specific OS. We gotta be as generic as possible.

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

I genuinely appreciate the time you've put into this and the time for the code reviews. This is a valuable contribution.

Having that said, I don't think this should land as it stands. At the very least, I don't think we should have "WSL" references to the main Node.js website. I don't see other runtimes such as Python or PHP or Go to even mention WSL AT ALL on their websites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants