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.
Minimum reproduction code
N/A
Steps to reproduce
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
nest-cli/actions/info.action.ts
Line 4 in 609bb5c
nest-cli/actions/info.action.ts
Line 42 in 609bb5c
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π
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:
- https://github.com/sindresorhus/macos-release/blob/main/index.js
- https://github.com/sindresorhus/windows-release/blob/main/index.js
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