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.

Sign up for GitHub

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

Jump to bottom

fix(cli): patch missing SYSTEMROOT env var on windows #4221

Merged
merged 1 commit into from Mar 1, 2023
Merged

fix(cli): patch missing SYSTEMROOT env var on windows #4221

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/@sanity/cli/src/cli.ts
View file
Open in desktop
@@ -1,4 +1,5 @@
/* eslint-disable no-console, no-process-exit, no-sync */
import os from 'os'
import path from 'path'
import {existsSync} from 'fs'
import chalk from 'chalk'
Expand Down Expand Up @@ -36,6 +37,7 @@ export async function runCli(cliRoot: string, {cliVersion}: {cliVersion: string}
}

loadAndSetEnvFromDotEnvFiles({workDir, cmd: args.groupOrCommand})
maybeFixMissingWindowsEnvVar()

// Check if there are updates available for the CLI, and notify if there is
await runUpdateCheck({pkg, cwd, workDir}).notify()
Expand Down Expand Up @@ -240,3 +242,21 @@ function loadAndSetEnvFromDotEnvFiles({workDir, cmd}: {workDir: string; cmd: str
process.env = {...process.env, ...studioEnv}
/* eslint-disable no-process-env */
}

/**
* Apparently, Windows environment variables are supposed to be case-insensitive,
* (https://nodejs.org/api/process.html#processenv). However, it seems they are not?
* `process.env.SYSTEMROOT` is sometimes `undefined`, whereas `process.env.SystemRoot` is _NOT_.
*
* The `open` npm module uses the former to open a browser on Powershell, and Sindre seems
* unwilling to fix it (https://github.com/sindresorhus/open/pull/299#issuecomment-1447587598),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not what I said. I said I wanted to see an issue posted on Node.js first, so it can be properly fixed eventually. When that's done, I'm happy to merge the workaround.

* so this is a (temporary?) workaround in order to make opening browsers on windows work,
* which several commands does (`sanity login`, `sanity docs` etc)
*/
function maybeFixMissingWindowsEnvVar() {
/* eslint-disable no-process-env */
if (os.platform() === 'win32' && !('SYSTEMROOT' in process.env) && 'SystemRoot' in process.env) {
process.env.SYSTEMROOT = process.env.SystemRoot
}
/* eslint-enable no-process-env */
}