fix(build): Stop scripts from trying to read user input during install

__tests__/commands/install/__snapshots__/integration.js.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`install should not hoist packages above their peer dependencies: install-file-as-default-no-semver 1`] = `
+exports[`don't install with file: protocol as default if target is valid semver: install-file-as-default-no-semver 1`] = `
 "{
   \\"author\\": \\"AJ ONeal <coolaj86@gmail.com> (http://coolaj86.info)\\",
   \\"name\\": \\"foo\\",

__tests__/commands/install/integration.js

@@ -492,14 +492,20 @@ test.concurrent('install with explicit file: protocol if target does not have pa
   });
 });
 
-test.concurrent("don't install with file: protocol as default if target is valid semver", (): Promise<void> => {
+test("don't install with file: protocol as default if target is valid semver", (): Promise<void> => {
   return runInstall({}, 'install-file-as-default-no-semver', async config => {
     expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'package.json'))).toMatchSnapshot(
       'install-file-as-default-no-semver',
     );
   });
 });
 
+test.concurrent("don't hang when an install script tries to read from stdin", (): Promise<void> =>
+  runInstall({}, 'install-blocking-script', (_config, _reporter, _install, getStdout) =>
+    expect(getStdout()).toMatch(/Building fresh packages/),
+  ),
+);
+
 // When local packages are installed, dependencies with different forms of the same relative path
 // should be deduped e.g. 'file:b' and 'file:./b'
 test.concurrent('install file: dedupe dependencies 1', (): Promise<void> => {

__tests__/commands/run.js

@@ -50,8 +50,8 @@ const runRunWithCustomShell = function(customShell, ...args): Promise<void> {
   })(...args);
 };
 
-test('lists all available commands with no arguments', (): Promise<void> => {
-  return runRun([], {}, 'no-args', (config, reporter): ?Promise<void> => {
+test('lists all available commands with no arguments', (): Promise<void> =>
+  runRun([], {}, 'no-args', (config, reporter): ?Promise<void> => {
     const rprtr = new reporters.BufferReporter({stdout: null, stdin: null});
     const scripts = ['build', 'prestart', 'start'];
     const hints = {
@@ -69,43 +69,48 @@ test('lists all available commands with no arguments', (): Promise<void> => {
     rprtr.error(rprtr.lang('commandNotSpecified'));
 
     expect(reporter.getBuffer()).toEqual(rprtr.getBuffer());
-  });
-});
+  }));
 
-test('runs script containing spaces', (): Promise<void> => {
-  return runRun(['build'], {}, 'spaces', async (config): ?Promise<void> => {
+test('runs script containing spaces', (): Promise<void> =>
+  runRun(['build'], {}, 'spaces', async (config): ?Promise<void> => {
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
     // The command gets called with a space appended
-    const args = ['build', config, pkg.scripts.build, config.cwd];
-
-    expect(execCommand).toBeCalledWith(...args);
-  });
-});
+    expect(execCommand).toBeCalledWith({
+      stage: 'build',
+      config,
+      cmd: pkg.scripts.build,
+      cwd: config.cwd,
+      isInteractive: true,
+    });
+  }));
 
-test('properly handles extra arguments and pre/post scripts', (): Promise<void> => {
-  return runRun(['start', '--hello'], {}, 'extra-args', async (config): ?Promise<void> => {
+test('properly handles extra arguments and pre/post scripts', (): Promise<void> =>
+  runRun(['start', '--hello'], {}, 'extra-args', async (config): ?Promise<void> => {
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
-    const poststart = ['poststart', config, pkg.scripts.poststart, config.cwd];
-    const prestart = ['prestart', config, pkg.scripts.prestart, config.cwd];
-    const start = ['start', config, pkg.scripts.start + ' --hello', config.cwd];
-
-    expect(execCommand.mock.calls[0]).toEqual(prestart);
-    expect(execCommand.mock.calls[1]).toEqual(start);
-    expect(execCommand.mock.calls[2]).toEqual(poststart);
-  });
-});
+    const poststart = {stage: 'poststart', config, cmd: pkg.scripts.poststart, cwd: config.cwd, isInteractive: true};
+    const prestart = {stage: 'prestart', config, cmd: pkg.scripts.prestart, cwd: config.cwd, isInteractive: true};
+    const start = {stage: 'start', config, cmd: pkg.scripts.start + ' --hello', cwd: config.cwd, isInteractive: true};
 
-test('properly handle bin scripts', (): Promise<void> => {
-  return runRun(['cat-names'], {}, 'bin', config => {
+    expect(execCommand.mock.calls[0]).toEqual([prestart]);
+    expect(execCommand.mock.calls[1]).toEqual([start]);
+    expect(execCommand.mock.calls[2]).toEqual([poststart]);
+  }));
+
+test('properly handle bin scripts', (): Promise<void> =>
+  runRun(['cat-names'], {}, 'bin', config => {
     const script = path.join(config.cwd, 'node_modules', '.bin', 'cat-names');
-    const args = ['cat-names', config, script, config.cwd];
 
-    expect(execCommand).toBeCalledWith(...args);
-  });
-});
+    expect(execCommand).toBeCalledWith({
+      stage: 'cat-names',
+      config,
+      cmd: script,
+      cwd: config.cwd,
+      isInteractive: true,
+    });
+  }));
 
-test('properly handle env command', (): Promise<void> => {
-  return runRun(['env'], {}, 'no-args', (config, reporter): ?Promise<void> => {
+test('properly handle env command', (): Promise<void> =>
+  runRun(['env'], {}, 'no-args', (config, reporter): ?Promise<void> => {
     // $FlowFixMe
     const result = JSON.parse(reporter.getBuffer()[0].data);
 
@@ -134,58 +139,66 @@ test('properly handle env command', (): Promise<void> => {
     expect(result).toHaveProperty('npm_lifecycle_event');
     expect(result).toHaveProperty('npm_execpath');
     expect(result).toHaveProperty('npm_node_execpath');
-  });
-});
+  }));
 
-test('adds string delimiters if args have spaces', (): Promise<void> => {
-  return runRun(['cat-names', '--filter', 'cat names'], {}, 'bin', config => {
+test('adds string delimiters if args have spaces', (): Promise<void> =>
+  runRun(['cat-names', '--filter', 'cat names'], {}, 'bin', config => {
     const script = path.join(config.cwd, 'node_modules', '.bin', 'cat-names');
     const q = process.platform === 'win32' ? '"' : "'";
-    const args = ['cat-names', config, `${script} --filter ${q}cat names${q}`, config.cwd];
 
-    expect(execCommand).toBeCalledWith(...args);
-  });
-});
+    expect(execCommand).toBeCalledWith({
+      stage: 'cat-names',
+      config,
+      cmd: `${script} --filter ${q}cat names${q}`,
+      cwd: config.cwd,
+      isInteractive: true,
+    });
+  }));
 
-test('adds quotes if args have spaces and quotes', (): Promise<void> => {
-  return runRun(['cat-names', '--filter', '"cat names"'], {}, 'bin', config => {
+test('adds quotes if args have spaces and quotes', (): Promise<void> =>
+  runRun(['cat-names', '--filter', '"cat names"'], {}, 'bin', config => {
     const script = path.join(config.cwd, 'node_modules', '.bin', 'cat-names');
     const quotedCatNames = process.platform === 'win32' ? '^"\\^"cat^ names\\^"^"' : `'"cat names"'`;
-    const args = ['cat-names', config, `${script} --filter ${quotedCatNames}`, config.cwd];
 
-    expect(execCommand).toBeCalledWith(...args);
-  });
-});
+    expect(execCommand).toBeCalledWith({
+      stage: 'cat-names',
+      config,
+      cmd: `${script} --filter ${quotedCatNames}`,
+      cwd: config.cwd,
+      isInteractive: true,
+    });
+  }));
 
-test('returns noScriptsAvailable with no scripts', (): Promise<void> => {
-  return runRun([], {}, 'no-scripts', (config, reporter) => {
+test('returns noScriptsAvailable with no scripts', (): Promise<void> =>
+  runRun([], {}, 'no-scripts', (config, reporter) => {
     expect(reporter.getBuffer()).toMatchSnapshot();
-  });
-});
+  }));
 
-test('returns noBinAvailable with no bins', (): Promise<void> => {
-  return runRun([], {}, 'no-bin', (config, reporter) => {
+test('returns noBinAvailable with no bins', (): Promise<void> =>
+  runRun([], {}, 'no-bin', (config, reporter) => {
     expect(reporter.getBuffer()).toMatchSnapshot();
-  });
-});
+  }));
 
-test('adds workspace root node_modules/.bin to path when in a workspace', (): Promise<void> => {
-  return runRunInWorkspacePackage('packages/pkg1', ['env'], {}, 'workspace', (config, reporter): ?Promise<void> => {
+test('adds workspace root node_modules/.bin to path when in a workspace', (): Promise<void> =>
+  runRunInWorkspacePackage('packages/pkg1', ['env'], {}, 'workspace', (config, reporter): ?Promise<void> => {
     const logEntry = reporter.getBuffer().find(entry => entry.type === 'log');
     const parsedLogData = JSON.parse(logEntry ? logEntry.data.toString() : '{}');
     const envPaths = (parsedLogData.PATH || parsedLogData.Path).split(path.delimiter);
 
     expect(envPaths).toContain(path.join(config.cwd, 'node_modules', '.bin'));
     expect(envPaths).toContain(path.join(config.cwd, 'packages', 'pkg1', 'node_modules', '.bin'));
-  });
-});
+  }));
 
-test('runs script with custom script-shell', (): Promise<void> => {
-  return runRunWithCustomShell('/usr/bin/dummy', ['start'], {}, 'script-shell', async (config): ?Promise<void> => {
+test('runs script with custom script-shell', (): Promise<void> =>
+  runRunWithCustomShell('/usr/bin/dummy', ['start'], {}, 'script-shell', async (config): ?Promise<void> => {
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
     // The command gets called with the provided customShell
-    const args = ['start', config, pkg.scripts.start, config.cwd, '/usr/bin/dummy'];
-
-    expect(execCommand).toBeCalledWith(...args);
-  });
-});
+    expect(execCommand).toBeCalledWith({
+      stage: 'start',
+      config,
+      cmd: pkg.scripts.start,
+      cwd: config.cwd,
+      isInteractive: true,
+      customShell: '/usr/bin/dummy',
+    });
+  }));

__tests__/commands/version.js

@@ -75,29 +75,59 @@ test('run version and make sure all lifecycle steps are executed', (): Promise<v
   return runRun([], {newVersion, gitTagVersion}, 'no-args', async (config): ?Promise<void> => {
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-    const preversionLifecycle = ['preversion', config, pkg.scripts.preversion, config.cwd];
-    const versionLifecycle = ['version', config, pkg.scripts.version, config.cwd];
-    const postversionLifecycle = ['postversion', config, pkg.scripts.postversion, config.cwd];
+    const preversionLifecycle = {
+      stage: 'preversion',
+      config,
+      cmd: pkg.scripts.preversion,
+      cwd: config.cwd,
+      isInteractive: true,
+    };
+    const versionLifecycle = {
+      stage: 'version',
+      config,
+      cmd: pkg.scripts.version,
+      cwd: config.cwd,
+      isInteractive: true,
+    };
+    const postversionLifecycle = {
+      stage: 'postversion',
+      config,
+      cmd: pkg.scripts.postversion,
+      cwd: config.cwd,
+      isInteractive: true,
+    };
 
     expect(execCommand.mock.calls.length).toBe(3);
 
-    expect(execCommand.mock.calls[0]).toEqual(preversionLifecycle);
-    expect(execCommand.mock.calls[1]).toEqual(versionLifecycle);
-    expect(execCommand.mock.calls[2]).toEqual(postversionLifecycle);
+    expect(execCommand.mock.calls[0]).toEqual([preversionLifecycle]);
+    expect(execCommand.mock.calls[1]).toEqual([versionLifecycle]);
+    expect(execCommand.mock.calls[2]).toEqual([postversionLifecycle]);
   });
 });
 
 test('run version and make sure only the defined lifecycle steps are executed', (): Promise<void> => {
   return runRun([], {newVersion, gitTagVersion}, 'pre-post', async (config): ?Promise<void> => {
     const pkg = await fs.readJson(path.join(config.cwd, 'package.json'));
 
-    const preversionLifecycle = ['preversion', config, pkg.scripts.preversion, config.cwd];
-    const postversionLifecycle = ['postversion', config, pkg.scripts.postversion, config.cwd];
+    const preversionLifecycle = {
+      stage: 'preversion',
+      config,
+      cmd: pkg.scripts.preversion,
+      cwd: config.cwd,
+      isInteractive: true,
+    };
+    const postversionLifecycle = {
+      stage: 'postversion',
+      config,
+      cmd: pkg.scripts.postversion,
+      cwd: config.cwd,
+      isInteractive: true,
+    };
 
     expect(execCommand.mock.calls.length).toBe(2);
 
-    expect(execCommand.mock.calls[0]).toEqual(preversionLifecycle);
-    expect(execCommand.mock.calls[1]).toEqual(postversionLifecycle);
+    expect(execCommand.mock.calls[0]).toEqual([preversionLifecycle]);
+    expect(execCommand.mock.calls[1]).toEqual([postversionLifecycle]);
   });
 });
 

__tests__/fixtures/install/install-blocking-script/blocking/install.js

@@ -0,0 +1,25 @@
+const readline = require('readline');
+const rl = readline.createInterface({
+  input: process.stdin,
+  output: process.stdout,
+  prompt: 'OHAI> ',
+});
+
+rl.prompt();
+
+rl
+  .on('line', line => {
+    switch (line.trim()) {
+      case 'hello':
+        console.log('world!');
+        break;
+      default:
+        console.log(`Say what? I might have heard '${line.trim()}'`);
+        break;
+    }
+    rl.prompt();
+  })
+  .on('close', () => {
+    console.log('Have a great day!');
+    process.exit(0);
+  });

__tests__/fixtures/install/install-blocking-script/blocking/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "blocking",
+  "version": "0.0.0",
+  "scripts": {
+    "install": "node install.js"
+  }
+}

__tests__/fixtures/install/install-blocking-script/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "blocking": "file:blocking"
+  }
+}

src/cli/commands/run.js

@@ -82,11 +82,14 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
         // only tack on trailing arguments for default script, ignore for pre and post - #1595
         const cmdWithArgs = stage === action ? sh`${unquoted(cmd)} ${args}` : cmd;
         const customShell = config.getOption('script-shell');
-        if (customShell) {
-          await execCommand(stage, config, cmdWithArgs, config.cwd, String(customShell));
-        } else {
-          await execCommand(stage, config, cmdWithArgs, config.cwd);
-        }
+        await execCommand({
+          stage,
+          config,
+          cmd: cmdWithArgs,
+          cwd: config.cwd,
+          isInteractive: true,
+          customShell: customShell ? String(customShell) : undefined,
+        });
       }
     } else if (action === 'env') {
       reporter.log(JSON.stringify(await makeEnv('env', config.cwd, config), null, 2), {force: true});

src/cli/commands/version.js

@@ -52,7 +52,7 @@ export async function setVersion(
 
   function runLifecycle(lifecycle: string): Promise<void> {
     if (scripts[lifecycle]) {
-      return execCommand(lifecycle, config, scripts[lifecycle], config.cwd);
+      return execCommand({stage: lifecycle, config, cmd: scripts[lifecycle], cwd: config.cwd, isInteractive: true});
     }
 
     return Promise.resolve();