Fix systemroot inconsistent casing (Fixes #292)

sindresorhus / open

Open stuff like URLs, files, executables. Cross-platform.

MIT License

3.14k stars 213 forks source link

Fix systemroot inconsistent casing (Fixes #292) #299

Closed Codex- closed 1 year ago

Codex- commented 1 year ago

Long time no PR, sindresorhus 😄

This PR simply adds a fallback for casing differences in SYSTEMROOT across windows versions, currently when using this on W11 (at least the 3 versions I tested) SYSTEMROOT was undefined, however SystemRoot is defined.

So, try and use both

Fixes #292

sindresorhus commented 1 year ago

On Windows operating systems, environment variables are case-insensitive.

https://nodejs.org/api/process.html#processenv

Codex- commented 1 year ago

I read that too, however it doesn't seem to always be the case for all env vars:

PS C:\dev\bfp-app\packages\app> node -v    
v16.19.1
PS C:\dev\bfp-app\packages\app> npm -v     
9.5.1

I don't normally develop on windows, so this is on a fresh installation of w11 on a VM too

sindresorhus commented 1 year ago

Then open an issue on Node.js and add a code comment above your code change with a link to the Node.js issue. I prefer to only add workarounds when there's a chance it will be fixed properly at some point.

Codex- commented 1 year ago

No problem, thanks anyway

sindresorhus commented 1 year ago

Why are you closing?

Codex- commented 1 year ago

Why are you closing?

I'm unable to consistently reproduce this in a blank sandboxe, so I'm assuming that it must be some combination of our code and environment causing this as opposed to node or otherwise. If I find a more consistent reproduction in node, I'll re-open with an issue, until then I'm going to assume it's a me problem :)