fix(core): repair SIGINT signals on windows (#28496)

using `windowsHide: true` is causing an issue on windows: Ctrl + C handling isn't enabled and no `SIGINT` is sent to the child process when users exit the process. See nodejs/node#29837 and nodejs/node-v0.x-archive#5054 for reference. This will cause leftover processes throughout nx. This PR sets `windowsHide: false` everywhere except for the plugin workers and some short-lived utils. They `spawn` child processes but have explicit handling to make sure they kill themselves when the parent process dies, so the missing Ctrl + C handling doesn't cause issues. We will follow up to make sure any other culprits that still cause windows popups (especially when used through Nx Console) are handled. Leaving no leftover processes running is more important for now, though. Keep in mind the underlying tooling (like vite) might have some windows popups themselves that Nx will inherit.
nrwl · Oct 17, 2024 · 499300f · 499300f
1 parent 42da542
commit 499300f
Show file tree

Hide file tree

Showing 87 changed files with 179 additions and 173 deletions.
diff --git a/e2e/utils/command-utils.ts b/e2e/utils/command-utils.ts
@@ -273,7 +273,7 @@ export function runCommandUntil(
       ...opts.env,
       FORCE_COLOR: 'false',
     },
-    windowsHide: true,
+    windowsHide: false,
   });
   return new Promise((res, rej) => {
     let output = '';

diff --git a/e2e/utils/global-setup.ts b/e2e/utils/global-setup.ts
@@ -65,7 +65,7 @@ function getPublishedVersion(): Promise<string | undefined> {
     exec(
       'npm view nx@latest version',
       {
-        windowsHide: true,
+        windowsHide: false,
       },
       (error, stdout, stderr) => {
         if (error) {

diff --git a/packages/create-nx-workspace/src/utils/child-process-utils.ts b/packages/create-nx-workspace/src/utils/child-process-utils.ts
@@ -19,7 +19,7 @@ export function spawnAndWait(command: string, args: string[], cwd: string) {
         ESLINT_USE_FLAT_CONFIG: process.env.ESLINT_USE_FLAT_CONFIG ?? 'true',
       },
       shell: true,
-      windowsHide: true,
+      windowsHide: false,
     });
 
     childProcess.on('exit', (code) => {
@@ -36,7 +36,7 @@ export function execAndWait(command: string, cwd: string) {
   return new Promise<{ code: number; stdout: string }>((res, rej) => {
     exec(
       command,
-      { cwd, env: { ...process.env, NX_DAEMON: 'false' }, windowsHide: true },
+      { cwd, env: { ...process.env, NX_DAEMON: 'false' }, windowsHide: false },
       (error, stdout, stderr) => {
         if (error) {
           const logFile = join(cwd, 'error.log');

diff --git a/packages/create-nx-workspace/src/utils/git/default-base.ts b/packages/create-nx-workspace/src/utils/git/default-base.ts
@@ -8,7 +8,7 @@ export function deduceDefaultBase(): string {
   const nxDefaultBase = 'main';
   try {
     return (
-      execSync('git config --get init.defaultBranch', { windowsHide: true })
+      execSync('git config --get init.defaultBranch', { windowsHide: false })
         .toString()
         .trim() || nxDefaultBase
     );

diff --git a/packages/create-nx-workspace/src/utils/git/git.ts b/packages/create-nx-workspace/src/utils/git/git.ts
@@ -4,7 +4,7 @@ import { output } from '../output';
 
 export function checkGitVersion(): string | null | undefined {
   try {
-    let gitVersionOutput = execSync('git --version', { windowsHide: true })
+    let gitVersionOutput = execSync('git --version', { windowsHide: false })
       .toString()
       .trim();
     return gitVersionOutput.match(/[0-9]+\.[0-9]+\.+[0-9]+/)?.[0];
@@ -43,7 +43,7 @@ export async function initializeGitRepo(
             }
           : {}),
       },
-      windowsHide: true,
+      windowsHide: false,
     };
     return new Promise<void>((resolve, reject) => {
       spawn('git', args, spawnOptions).on('close', (code) => {

diff --git a/packages/create-nx-workspace/src/utils/nx/ab-testing.ts b/packages/create-nx-workspace/src/utils/nx/ab-testing.ts
@@ -139,7 +139,7 @@ function shouldRecordStats(): boolean {
   try {
     const stdout = execSync(pmc.getRegistryUrl, {
       encoding: 'utf-8',
-      windowsHide: true,
+      windowsHide: false,
     });
     const url = new URL(stdout.trim());
 

diff --git a/packages/create-nx-workspace/src/utils/nx/show-nx-warning.ts b/packages/create-nx-workspace/src/utils/nx/show-nx-warning.ts
@@ -9,7 +9,7 @@ export function showNxWarning(workspaceName: string) {
     execSync('nx --version', {
       cwd: pathToRunNxCommand,
       stdio: ['ignore', 'ignore', 'ignore'],
-      windowsHide: true,
+      windowsHide: false,
     });
   } catch (e) {
     // no nx found

diff --git a/packages/create-nx-workspace/src/utils/package-manager.ts b/packages/create-nx-workspace/src/utils/package-manager.ts
@@ -123,7 +123,7 @@ export function getPackageManagerVersion(
   const version = execSync(`${packageManager} --version`, {
     cwd,
     encoding: 'utf-8',
-    windowsHide: true,
+    windowsHide: false,
   }).trim();
   pmVersionCache.set(packageManager, version);
   return version;

diff --git a/packages/cypress/plugins/cypress-preset.ts b/packages/cypress/plugins/cypress-preset.ts
@@ -78,14 +78,14 @@ function startWebServer(webServerCommand: string) {
     // Windows is fine so we leave it attached to this process
     detached: process.platform !== 'win32',
     stdio: 'inherit',
-    windowsHide: true,
+    windowsHide: false,
   });
 
   return () => {
     if (process.platform === 'win32') {
       try {
         execSync('taskkill /pid ' + serverProcess.pid + ' /T /F', {
-          windowsHide: true,
+          windowsHide: false,
         });
       } catch (e) {
         if (process.env.NX_VERBOSE_LOGGING === 'true') {

diff --git a/packages/devkit/src/tasks/install-packages-task.ts b/packages/devkit/src/tasks/install-packages-task.ts
@@ -43,7 +43,7 @@ export function installPackagesTask(
     const execSyncOptions: ExecSyncOptions = {
       cwd: join(tree.root, cwd),
       stdio: process.env.NX_GENERATE_QUIET === 'true' ? 'ignore' : 'inherit',
-      windowsHide: true,
+      windowsHide: false,
     };
     // ensure local registry from process is not interfering with the install
     // when we start the process from temp folder the local registry would override the custom registry

diff --git a/packages/devkit/src/utils/package-json.ts b/packages/devkit/src/utils/package-json.ts
@@ -497,7 +497,7 @@ export function ensurePackage<T extends any = any>(
     execSync(preInstallCommand, {
       cwd: tempDir,
       stdio: isVerbose ? 'inherit' : 'ignore',
-      windowsHide: true,
+      windowsHide: false,
     });
   }
   let addCommand = getPackageManagerCommand(packageManager).addDev;
@@ -508,7 +508,7 @@ export function ensurePackage<T extends any = any>(
   execSync(`${addCommand} ${pkg}@${requiredVersion}`, {
     cwd: tempDir,
     stdio: isVerbose ? 'inherit' : 'ignore',
-    windowsHide: true,
+    windowsHide: false,
   });
 
   addToNodePath(join(workspaceRoot, 'node_modules'));

diff --git a/packages/expo/src/utils/pod-install-task.ts b/packages/expo/src/utils/pod-install-task.ts
@@ -68,7 +68,7 @@ export function podInstall(
       execSync('touch .xcode.env', {
         cwd: iosDirectory,
         stdio: 'inherit',
-        windowsHide: true,
+        windowsHide: false,
       });
     }
     execSync(
@@ -78,7 +78,7 @@ export function podInstall(
       {
         cwd: iosDirectory,
         stdio: 'inherit',
-        windowsHide: true,
+        windowsHide: false,
       }
     );
   } catch (e) {

diff --git a/packages/expo/src/utils/resolve-eas.ts b/packages/expo/src/utils/resolve-eas.ts
@@ -11,13 +11,13 @@ export function resolveEas(workspaceRoot: string): string {
 
   let npmGlobalPath: string, yarnGlobalPath: string;
   try {
-    npmGlobalPath = execSync('npm root -g', { windowsHide: true })
+    npmGlobalPath = execSync('npm root -g', { windowsHide: false })
       ?.toString()
       ?.trim()
       ?.replace('\u001b[2K\u001b[1G', ''); // strip out ansi codes
   } catch {}
   try {
-    yarnGlobalPath = execSync('yarn global dir', { windowsHide: true })
+    yarnGlobalPath = execSync('yarn global dir', { windowsHide: false })
       ?.toString()
       ?.trim()
       ?.replace('\u001b[2K\u001b[1G', ''); // strip out ansi codes

diff --git a/packages/js/src/executors/node/lib/kill-tree.ts b/packages/js/src/executors/node/lib/kill-tree.ts
@@ -21,7 +21,7 @@ export async function killTree(pid: number, signal: NodeJS.Signals) {
         exec(
           'taskkill /pid ' + pid + ' /T /F',
           {
-            windowsHide: true,
+            windowsHide: false,
           },
           (error) => {
             // Ignore Fatal errors (128) because it might be due to the process already being killed.
@@ -37,7 +37,7 @@ export async function killTree(pid: number, signal: NodeJS.Signals) {
           pidsToProcess,
           function (parentPid) {
             return spawn('pgrep', ['-P', parentPid], {
-              windowsHide: true,
+              windowsHide: false,
             });
           },
           function () {
@@ -55,7 +55,7 @@ export async function killTree(pid: number, signal: NodeJS.Signals) {
               'ps',
               ['-o', 'pid', '--no-headers', '--ppid', parentPid],
               {
-                windowsHide: true,
+                windowsHide: false,
               }
             );
           },

diff --git a/packages/js/src/executors/release-publish/release-publish.impl.ts b/packages/js/src/executors/release-publish/release-publish.impl.ts
@@ -128,7 +128,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
         env: processEnv(true),
         cwd: context.root,
         stdio: ['ignore', 'pipe', 'pipe'],
-        windowsHide: true,
+        windowsHide: false,
       });
 
       const resultJson = JSON.parse(result.toString());
@@ -154,7 +154,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
               env: processEnv(true),
               cwd: context.root,
               stdio: 'ignore',
-              windowsHide: true,
+              windowsHide: false,
             });
             console.log(
               `Added the dist-tag ${tag} to v${currentVersion} for registry ${registry}.\n`
@@ -269,7 +269,7 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
       env: processEnv(true),
       cwd: context.root,
       stdio: ['ignore', 'pipe', 'pipe'],
-      windowsHide: true,
+      windowsHide: false,
     });
 
     /**

diff --git a/packages/js/src/executors/verdaccio/verdaccio.impl.ts b/packages/js/src/executors/verdaccio/verdaccio.impl.ts
@@ -136,7 +136,7 @@ function createVerdaccioOptions(
 
 function setupNpm(options: VerdaccioExecutorSchema) {
   try {
-    execSync('npm --version', { env, windowsHide: true });
+    execSync('npm --version', { env, windowsHide: false });
   } catch (e) {
     return () => {};
   }
@@ -151,20 +151,20 @@ function setupNpm(options: VerdaccioExecutorSchema) {
         npmRegistryPaths.push(
           execSync(
             `npm config get ${registryName} --location ${options.location}`,
-            { env, windowsHide: true }
+            { env, windowsHide: false }
           )
             ?.toString()
             ?.trim()
             ?.replace('\u001b[2K\u001b[1G', '') // strip out ansi codes
         );
         execSync(
           `npm config set ${registryName} http://localhost:${options.port}/ --location ${options.location}`,
-          { env, windowsHide: true }
+          { env, windowsHide: false }
         );
 
         execSync(
           `npm config set //localhost:${options.port}/:_authToken="secretVerdaccioToken" --location ${options.location}`,
-          { env, windowsHide: true }
+          { env, windowsHide: false }
         );
 
         logger.info(
@@ -181,7 +181,7 @@ function setupNpm(options: VerdaccioExecutorSchema) {
       try {
         const currentNpmRegistryPath = execSync(
           `npm config get registry --location ${options.location}`,
-          { env, windowsHide: true }
+          { env, windowsHide: false }
         )
           ?.toString()
           ?.trim()
@@ -194,7 +194,7 @@ function setupNpm(options: VerdaccioExecutorSchema) {
           ) {
             execSync(
               `npm config set ${registryName} ${npmRegistryPaths[index]} --location ${options.location}`,
-              { env, windowsHide: true }
+              { env, windowsHide: false }
             );
             logger.info(
               `Reset npm ${registryName} to ${npmRegistryPaths[index]}`
@@ -204,15 +204,15 @@ function setupNpm(options: VerdaccioExecutorSchema) {
               `npm config delete ${registryName} --location ${options.location}`,
               {
                 env,
-                windowsHide: true,
+                windowsHide: false,
               }
             );
             logger.info('Cleared custom npm registry');
           }
         });
         execSync(
           `npm config delete //localhost:${options.port}/:_authToken  --location ${options.location}`,
-          { env, windowsHide: true }
+          { env, windowsHide: false }
         );
       } catch (e) {
         throw new Error(`Failed to reset npm registry: ${e.message}`);
@@ -231,7 +231,7 @@ function getYarnUnsafeHttpWhitelist(isYarnV1: boolean) {
         JSON.parse(
           execSync(`yarn config get unsafeHttpWhitelist --json`, {
             env,
-            windowsHide: true,
+            windowsHide: false,
           }).toString()
         )
       )
@@ -247,13 +247,13 @@ function setYarnUnsafeHttpWhitelist(
       `yarn config set unsafeHttpWhitelist --json '${JSON.stringify(
         Array.from(currentWhitelist)
       )}'` + (options.location === 'user' ? ' --home' : ''),
-      { env, windowsHide: true }
+      { env, windowsHide: false }
     );
   } else {
     execSync(
       `yarn config unset unsafeHttpWhitelist` +
         (options.location === 'user' ? ' --home' : ''),
-      { env, windowsHide: true }
+      { env, windowsHide: false }
     );
   }
 }
@@ -266,7 +266,9 @@ function setupYarn(options: VerdaccioExecutorSchema) {
   try {
     isYarnV1 =
       major(
-        execSync('yarn --version', { env, windowsHide: true }).toString().trim()
+        execSync('yarn --version', { env, windowsHide: false })
+          .toString()
+          .trim()
       ) === 1;
   } catch {
     // This would fail if yarn is not installed which is okay
@@ -281,7 +283,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
       yarnRegistryPaths.push(
         execSync(`yarn config get ${scopeName}${registryConfigName}`, {
           env,
-          windowsHide: true,
+          windowsHide: false,
         })
           ?.toString()
           ?.trim()
@@ -291,7 +293,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
       execSync(
         `yarn config set ${scopeName}${registryConfigName} http://localhost:${options.port}/` +
           (options.location === 'user' ? ' --home' : ''),
-        { env, windowsHide: true }
+        { env, windowsHide: false }
       );
 
       logger.info(
@@ -318,7 +320,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
       try {
         const currentYarnRegistryPath = execSync(
           `yarn config get ${registryConfigName}`,
-          { env, windowsHide: true }
+          { env, windowsHide: false }
         )
           ?.toString()
           ?.trim()
@@ -339,7 +341,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
               {
                 env,
 
-                windowsHide: true,
+                windowsHide: false,
               }
             );
             logger.info(
@@ -349,7 +351,7 @@ function setupYarn(options: VerdaccioExecutorSchema) {
             execSync(
               `yarn config ${isYarnV1 ? 'delete' : 'unset'} ${registryName}` +
                 (options.location === 'user' ? ' --home' : ''),
-              { env, windowsHide: true }
+              { env, windowsHide: false }
             );
             logger.info(`Cleared custom yarn ${registryConfigName}`);
           }

diff --git a/packages/js/src/generators/release-version/release-version.ts b/packages/js/src/generators/release-version/release-version.ts
@@ -213,7 +213,7 @@ To fix this you will either need to add a package.json file at that location, or
                 exec(
                   `npm view ${packageName} version --"${registryConfigKey}=${registry}" --tag=${tag}`,
                   {
-                    windowsHide: true,
+                    windowsHide: false,
                   },
                   (error, stdout, stderr) => {
                     if (error) {

diff --git a/packages/js/src/generators/release-version/utils/update-lock-file.ts b/packages/js/src/generators/release-version/utils/update-lock-file.ts
@@ -133,7 +133,7 @@ function execLockFileUpdate(
         ...process.env,
         ...env,
       },
-      windowsHide: true,
+      windowsHide: false,
     });
   } catch (e) {
     output.error({

diff --git a/packages/js/src/generators/setup-verdaccio/generator.ts b/packages/js/src/generators/setup-verdaccio/generator.ts
@@ -23,7 +23,7 @@ export async function setupVerdaccio(
     generateFiles(tree, path.join(__dirname, 'files'), '.verdaccio', {
       npmUplinkRegistry:
         execSync('npm config get registry', {
-          windowsHide: true,
+          windowsHide: false,
         })
           ?.toString()
           ?.trim() ?? 'https://registry.npmjs.org',