Skip to content

Commit

Permalink
Add missing "--terminal" argument to run-android (facebook#20584)
Browse files Browse the repository at this point in the history
Summary:
Commit 79e498b claims that "--terminal" has been added as an argument to the "run-android" script which it hasn't. Instead it adds a new environment variable "process.env.REACT_TERMINAL" which seems to be undocumented and nowhere else in the repository.

This commit now allows Linux and OSX users to specify their custom terminal :

    react-native run-android --terminal=x-terminal-emulator

Additionally the metro bundler child process "detached" option has been set to false if no terminal has been specified on Linux which fixes an issue where the packager would be running in the background and would have to manually be closed by finding the process.

Fixes facebook#18122
Pull Request resolved: facebook#20584

Differential Revision: D9234990

Pulled By: hramos

fbshipit-source-id: 60e4cc57b2790c419d5a4f9027add04313ddf6f8
  • Loading branch information
jamsch authored and hramos committed Aug 22, 2018
1 parent efa2997 commit 81fba40
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions local-cli/runAndroid/runAndroid.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function runAndroid(argv, config, args) {
} else {
// result == 'not_running'
console.log(chalk.bold('Starting JS server...'));
startServerInNewWindow(args.port);
startServerInNewWindow(args.port, args.terminal);
}
return buildAndRun(args);
});
Expand Down Expand Up @@ -348,7 +348,7 @@ function runOnAllDevices(
}
}

function startServerInNewWindow(port) {
function startServerInNewWindow(port, terminal = process.env.REACT_TERMINAL) {
// set up OS-specific filenames and commands
const isWindows = /^win/.test(process.platform);
const scriptFile = isWindows
Expand All @@ -363,7 +363,6 @@ function startServerInNewWindow(port) {
const scriptsDir = path.resolve(__dirname, '..', '..', 'scripts');
const launchPackagerScript = path.resolve(scriptsDir, scriptFile);
const procConfig = {cwd: scriptsDir};
const terminal = process.env.REACT_TERMINAL;

// set up the .packager.(env|bat) file to ensure the packager starts on the right port
const packagerEnvFile = path.join(
Expand All @@ -390,14 +389,16 @@ function startServerInNewWindow(port) {
}
return child_process.spawnSync('open', [launchPackagerScript], procConfig);
} else if (process.platform === 'linux') {
procConfig.detached = true;
if (terminal) {
procConfig.detached = true;
return child_process.spawn(
terminal,
['-e', 'sh ' + launchPackagerScript],
procConfig,
);
}
// By default, the child shell process will be attached to the parent
procConfig.detached = false;
return child_process.spawn('sh', [launchPackagerScript], procConfig);
} else if (/^win/.test(process.platform)) {
procConfig.detached = true;
Expand Down Expand Up @@ -474,5 +475,11 @@ module.exports = {
default: process.env.RCT_METRO_PORT || 8081,
parse: (val: string) => Number(val),
},
{
command: '--terminal [string]',
description:
'Launches the Metro Bundler in a new window using the specified terminal path.',
default: '',
},
],
};

0 comments on commit 81fba40

Please sign in to comment.