ionic-cli icon indicating copy to clipboard operation
ionic-cli copied to clipboard

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

Open jonz94 opened this issue 1 year ago • 0 comments

Closes #4961

This PR upgrades the os-name package from 4.0.0 to latest 5.1.0 (https://github.com/sindresorhus/os-name/compare/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 (https://github.com/jonz94/ionic-cli/commit/8b74ed400d39115f932e2d457ea1431736545a0f). 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 https://github.com/ionic-team/ionic-cli/blob/a2eb4b2cf46e0f2004c1850acfdeada46fb211b5/packages/%40ionic/cli/package.json#L10-L12 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.

jonz94 avatar Mar 16 '23 23:03 jonz94