nestjs/nest-cli

Unknown OS Version on newer os versions

Closed this issue Β· 16 comments

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

Running nest info command logs the operating system version as "macOS Unknown." for newer os versions which are not supported by os-name package.

Screenshot 2023-11-24 at 18 24 56

Minimum reproduction code

N/A

Steps to reproduce

  1. nest info

Expected behavior

macOS Sonoma should be logged as the OS Version.

Package version

10.2.1

NestJS version

10.2.10

Node.js version

21.1.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

perhaps we just need to upgrade os-name package that we're using to retrieve that version

import osName = require('os-name');

console.info('OS Version :', chalk.blue(osName(platform(), release())));

I don't have a mac to try the latest version

Yes we should upgrade os-name, i opened a Pull Request in os-name to update the packages that it depends on them to get OS name macos-release and windows-release, those packages are not updated frequently so i suggest to bundle that here instead of depending on those packages.

In fact, this problem is present even if you use macOS Ventura

@Tony133 Can you please try the latest version of that package. So we will see id we can anything here

@micalevisk it works fine with the latest os-name version 6.0.0

@micalevisk actually last year I had created a PR( see here:#1799) that updated the version of os-name to v5, only it was closed because from v5 onwards the package was updated to pure ESM ( see screenshot) . I sincerely didn't pay attention to it, Kamil pointed it out to me πŸ˜„ ( see his comment always in my PR), so you have to figure out how to proceedπŸ™‚

screenshot-os-name

Yeah and I don't think that the author will back port the fix to that old version

It is an idea, but what if we use the os package of node instead of os-name ?

We could do something like this( see code snippet ) print only the name of the operating system by doing a check via Node's os package. πŸ™‚

  if (os.platform == 'darwin') {
    return 'MacOS';
  } else if (os.platform == 'linux') {
    return 'Linux';
  } else if (os.platform == 'win32') {
    return 'Windows';
  }

we can use os.types() or os.platform(), so we remove a third party library.

@Tony133 this is not the same current output which is ex. Windows 8.1.

I believe that that package exists because of edge-cases

But I'm not sure. I've fixed this issue already but it would be better if we could drop that dependency, of course

@Tony133 this is not the same current output which is ex. Windows 8.1.

It is just an example but on windows I would not know because using macOS πŸ˜„ we would have to ask someone who uses windows.

Unix forever πŸ™Œ πŸ˜„

However, looking carefully at os-name inside it, it uses two libraries that check and name the macOS and Windows versions, as follows:

So, if we wanted, we could integrate something like this file: https://github.com/sindresorhus/os-name/blob/main/index.js
within the NestJS CLI, so that we could handle macOS or Windows versions independently.

@Tony133 i already suggested that, i think it's the best option to integrate this feature.

feel free to open a new PR with that version then. And Kamil will see which approach is better to him

I found a bit odd as we would still depend on those two packages anyway

@Tony133 i already suggested that, i think it's the best option to integrate this feature.

I missed the comment, sorry πŸ˜„ πŸ™πŸ»

I found a bit odd as we would still depend on those two packages anyway

yes, we can also handle the part of the two packages, is not even integrate them into the package.json, then it is only one file for macOS versions and one for Windows:

or we just print out just the operating system name as in the example snippet I wrote above is leave out the versions, etc.

Not thinks to be a bad idea to remove os-name package, however, I would say to wait for Kamil as soon as he finds some time to see what he thinks and which route to choose. πŸ™‚

Let's track this here #2403