Skip to content
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

fix(info): display correct os name on Windows 11 #4968

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jonz94
Copy link

@jonz94 jonz94 commented Mar 16, 2023

Closes #4961

This PR upgrades the os-name package from 4.0.0 to latest 5.1.0 (sindresorhus/os-name@v4.0.0...v5.1.0), which included a fix for Windows 11.


Note 1: As os-name v5 is pure ESM package, importing it with require('os-name') will result in an error. However, when "module": "commonjs" is set in tsconfig, tsc will automatically convert await import(...) into await Promise.resolve().then(() => require(...)):

image

which will cause the following error when running ionic info:

$ node C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\bin\ionic info
Error [ERR_REQUIRE_ESM]: require() of ES Module
C:\Users\jonz94\repos\ionic-repos\ionic-cli\node_modules\os-name\index.js from
C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\index.js not supported.

Instead change the require of C:\Users\jonz94\repos\ionic-repos\ionic-cli\node_modules\os-name\index.js in
C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\index.js to a dynamic import() which is available in
all CommonJS modules.
at C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\index.js:42:72
at async Environment.getInfo (C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\index.js:42:37)
at async InfoCommand.run (C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\commands\info.js:30:24)
at async Promise.all (index 0)
at async InfoCommand.execute (C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\command.js:81:9)
at async Executor.run (C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\lib\executor.js:54:9)
at async Executor.execute
(C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli-framework\lib\executor.js:70:13)
at async Object.run (C:\Users\jonz94\repos\ionic-repos\ionic-cli\packages\@ionic\cli\index.js:110:9)

To fix this, the recommended approach is to set "module": "node16" in TypeScript 4.7 or later. During my attempt to upgrade TypeScript to 4.7, I encountered sereval type errors while running lerna run build. I was able to resolve most of them through find and replace, and managed to successfully build 15 out of 16 packages (jonz94@8b74ed4). However, the remaining errors were different, and I felt like I did not have enough knowledge about the codebase to address those errors.

In the end, I resorted to using eval() as a quick and dirty workaround 🙃


Note 2: The import() function requires node.js >= 13.2.0, and currently the minimal required node.js version is 10.3.0

"engines": {
"node": ">=10.3.0"
},

However, once PR #4965 (or #4972) is merged, the minimal required version will be bumped up to 16.0.0, which means this won't be an issue anymore.

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.

bug: ionic info reports Windows 10 on Windows 11 Pro
1 participant