Skip to content

Commit cd28840

Browse files
committed
Speed up editor detection on Windows
before ~600ms after ~300ms
1 parent df00d80 commit cd28840

File tree

3 files changed

+162
-95
lines changed

3 files changed

+162
-95
lines changed

packages/react-dev-utils/launchEditor.js

+155-94
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,59 @@
1111
const fs = require('fs');
1212
const path = require('path');
1313
const child_process = require('child_process');
14+
const EventEmitter = require('events').EventEmitter;
1415
const os = require('os');
1516
const chalk = require('chalk');
1617
const shellQuote = require('shell-quote');
1718

19+
// Inspired by https://github.com/rannn505/node-powershell
20+
const EOI = 'EOI';
21+
class PowerShell extends EventEmitter {
22+
constructor() {
23+
super();
24+
25+
this._proc = child_process.spawn(
26+
'powershell.exe',
27+
['-NoLogo', '-NoProfile', '-NoExit', '-Command', '-'],
28+
{
29+
stdio: 'pipe',
30+
}
31+
);
32+
33+
let output = [];
34+
this._proc.stdout.on('data', data => {
35+
if (data.indexOf(EOI) !== -1) {
36+
this.emit('resolve', output.join(''));
37+
output = [];
38+
} else {
39+
output.push(data);
40+
}
41+
});
42+
43+
this._proc.on('error', () => {
44+
this._proc = null;
45+
});
46+
}
47+
48+
invoke(cmd) {
49+
if (!this._proc) {
50+
return Promise.resolve('');
51+
}
52+
53+
return new Promise(resolve => {
54+
this.on('resolve', data => {
55+
resolve(data);
56+
this.removeAllListeners('resolve');
57+
});
58+
59+
this._proc.stdin.write(cmd);
60+
this._proc.stdin.write(os.EOL);
61+
this._proc.stdin.write(`echo ${EOI}`);
62+
this._proc.stdin.write(os.EOL);
63+
});
64+
}
65+
}
66+
1867
function isTerminalEditor(editor) {
1968
switch (editor) {
2069
case 'vim':
@@ -104,57 +153,65 @@ function getArgumentsForLineNumber(editor, fileName, lineNumber, workspace) {
104153
return [fileName];
105154
}
106155

107-
function guessEditor() {
108-
// Explicit config always wins
109-
if (process.env.REACT_EDITOR) {
110-
return shellQuote.parse(process.env.REACT_EDITOR);
156+
let powerShellAgent = null;
157+
function launchPowerShellAgent() {
158+
if (!powerShellAgent) {
159+
powerShellAgent = new PowerShell();
111160
}
161+
}
112162

113-
// Using `ps x` on OSX or `Get-Process` on Windows we can find out which editor is currently running.
114-
// Potentially we could use similar technique for Linux
115-
try {
116-
if (process.platform === 'darwin') {
117-
const output = child_process.execSync('ps x').toString();
118-
const processNames = Object.keys(COMMON_EDITORS_OSX);
119-
for (let i = 0; i < processNames.length; i++) {
120-
const processName = processNames[i];
121-
if (output.indexOf(processName) !== -1) {
122-
return [COMMON_EDITORS_OSX[processName]];
123-
}
124-
}
125-
} else if (process.platform === 'win32') {
126-
const output = child_process
127-
.execSync('powershell -Command "Get-Process | Select-Object Path"', {
128-
stdio: ['pipe', 'pipe', 'ignore'],
129-
})
130-
.toString();
131-
const runningProcesses = output.split('\r\n');
132-
for (let i = 0; i < runningProcesses.length; i++) {
133-
// `Get-Process` sometimes returns empty lines
134-
if (!runningProcesses[i]) {
135-
continue;
163+
function guessEditor() {
164+
return new Promise(resolve => {
165+
// Explicit config always wins
166+
if (process.env.REACT_EDITOR) {
167+
return resolve(shellQuote.parse(process.env.REACT_EDITOR));
168+
}
169+
170+
// Using `ps x` on OSX or `Get-Process` on Windows we can find out which editor is currently running.
171+
// Potentially we could use similar technique for Linux
172+
try {
173+
if (process.platform === 'darwin') {
174+
const output = child_process.execSync('ps x').toString();
175+
const processNames = Object.keys(COMMON_EDITORS_OSX);
176+
for (let i = 0; i < processNames.length; i++) {
177+
const processName = processNames[i];
178+
if (output.indexOf(processName) !== -1) {
179+
return resolve([COMMON_EDITORS_OSX[processName]]);
180+
}
136181
}
182+
} else if (process.platform === 'win32' && powerShellAgent) {
183+
return powerShellAgent
184+
.invoke('Get-Process | Select-Object Path')
185+
.then(output => {
186+
const runningProcesses = output.split('\r\n');
187+
for (let i = 0; i < runningProcesses.length; i++) {
188+
// `Get-Process` sometimes returns empty lines
189+
if (!runningProcesses[i]) {
190+
continue;
191+
}
137192

138-
const fullProcessPath = runningProcesses[i].trim();
139-
const shortProcessName = path.basename(fullProcessPath);
193+
const fullProcessPath = runningProcesses[i].trim();
194+
const shortProcessName = path.basename(fullProcessPath);
140195

141-
if (COMMON_EDITORS_WIN.indexOf(shortProcessName) !== -1) {
142-
return [fullProcessPath];
143-
}
196+
if (COMMON_EDITORS_WIN.indexOf(shortProcessName) !== -1) {
197+
return resolve([fullProcessPath]);
198+
}
199+
}
200+
});
144201
}
202+
} catch (error) {
203+
// Ignore...
145204
}
146-
} catch (error) {
147-
// Ignore...
148-
}
149205

150-
// Last resort, use old skool env vars
151-
if (process.env.VISUAL) {
152-
return [process.env.VISUAL];
153-
} else if (process.env.EDITOR) {
154-
return [process.env.EDITOR];
155-
}
206+
// Last resort, use old skool env vars
207+
if (process.env.VISUAL) {
208+
return resolve([process.env.VISUAL]);
209+
} else if (process.env.EDITOR) {
210+
return resolve([process.env.EDITOR]);
211+
}
156212

157-
return [null];
213+
return resolve([null]);
214+
});
158215
}
159216

160217
function printInstructions(fileName, errorMessage) {
@@ -195,64 +252,68 @@ function launchEditor(fileName, lineNumber) {
195252
return;
196253
}
197254

198-
let [editor, ...args] = guessEditor();
199-
if (!editor) {
200-
printInstructions(fileName, null);
201-
return;
202-
}
203-
204-
if (
205-
process.platform === 'linux' &&
206-
fileName.startsWith('/mnt/') &&
207-
/Microsoft/i.test(os.release())
208-
) {
209-
// Assume WSL / "Bash on Ubuntu on Windows" is being used, and
210-
// that the file exists on the Windows file system.
211-
// `os.release()` is "4.4.0-43-Microsoft" in the current release
212-
// build of WSL, see: https://github.com/Microsoft/BashOnWindows/issues/423#issuecomment-221627364
213-
// When a Windows editor is specified, interop functionality can
214-
// handle the path translation, but only if a relative path is used.
215-
fileName = path.relative('', fileName);
216-
}
255+
guessEditor().then(([editor, ...args]) => {
256+
if (!editor) {
257+
printInstructions(fileName, null);
258+
return;
259+
}
217260

218-
let workspace = null;
219-
if (lineNumber) {
220-
args = args.concat(
221-
getArgumentsForLineNumber(editor, fileName, lineNumber, workspace)
222-
);
223-
} else {
224-
args.push(fileName);
225-
}
261+
if (
262+
process.platform === 'linux' &&
263+
fileName.startsWith('/mnt/') &&
264+
/Microsoft/i.test(os.release())
265+
) {
266+
// Assume WSL / "Bash on Ubuntu on Windows" is being used, and
267+
// that the file exists on the Windows file system.
268+
// `os.release()` is "4.4.0-43-Microsoft" in the current release
269+
// build of WSL, see: https://github.com/Microsoft/BashOnWindows/issues/423#issuecomment-221627364
270+
// When a Windows editor is specified, interop functionality can
271+
// handle the path translation, but only if a relative path is used.
272+
fileName = path.relative('', fileName);
273+
}
226274

227-
if (_childProcess && isTerminalEditor(editor)) {
228-
// There's an existing editor process already and it's attached
229-
// to the terminal, so go kill it. Otherwise two separate editor
230-
// instances attach to the stdin/stdout which gets confusing.
231-
_childProcess.kill('SIGKILL');
232-
}
275+
let workspace = null;
276+
if (lineNumber) {
277+
args = args.concat(
278+
getArgumentsForLineNumber(editor, fileName, lineNumber, workspace)
279+
);
280+
} else {
281+
args.push(fileName);
282+
}
233283

234-
if (process.platform === 'win32') {
235-
// On Windows, launch the editor in a shell because spawn can only
236-
// launch .exe files.
237-
_childProcess = child_process.spawn(
238-
'cmd.exe',
239-
['/C', editor].concat(args),
240-
{ stdio: 'inherit' }
241-
);
242-
} else {
243-
_childProcess = child_process.spawn(editor, args, { stdio: 'inherit' });
244-
}
245-
_childProcess.on('exit', function(errorCode) {
246-
_childProcess = null;
284+
if (_childProcess && isTerminalEditor(editor)) {
285+
// There's an existing editor process already and it's attached
286+
// to the terminal, so go kill it. Otherwise two separate editor
287+
// instances attach to the stdin/stdout which gets confusing.
288+
_childProcess.kill('SIGKILL');
289+
}
247290

248-
if (errorCode) {
249-
printInstructions(fileName, '(code ' + errorCode + ')');
291+
if (process.platform === 'win32') {
292+
// On Windows, launch the editor in a shell because spawn can only
293+
// launch .exe files.
294+
_childProcess = child_process.spawn(
295+
'cmd.exe',
296+
['/C', editor].concat(args),
297+
{ stdio: 'inherit' }
298+
);
299+
} else {
300+
_childProcess = child_process.spawn(editor, args, { stdio: 'inherit' });
250301
}
251-
});
302+
_childProcess.on('exit', function(errorCode) {
303+
_childProcess = null;
252304

253-
_childProcess.on('error', function(error) {
254-
printInstructions(fileName, error.message);
305+
if (errorCode) {
306+
printInstructions(fileName, '(code ' + errorCode + ')');
307+
}
308+
});
309+
310+
_childProcess.on('error', function(error) {
311+
printInstructions(fileName, error.message);
312+
});
255313
});
256314
}
257315

258-
module.exports = launchEditor;
316+
module.exports = {
317+
launchEditor,
318+
launchPowerShellAgent,
319+
};

packages/react-error-overlay/middleware.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
'use strict';
1010

11-
const launchEditor = require('react-dev-utils/launchEditor');
11+
const { launchEditor } = require('react-dev-utils/launchEditor');
1212

1313
module.exports = function createLaunchEditorMiddleware() {
1414
return function launchEditorMiddleware(req, res, next) {

packages/react-scripts/scripts/start.js

+6
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ choosePort(HOST, DEFAULT_PORT)
8585
}
8686
console.log(chalk.cyan('Starting the development server...\n'));
8787
openBrowser(urls.localUrlForBrowser);
88+
if (process.platform === 'win32') {
89+
const {
90+
launchPowerShellAgent,
91+
} = require('react-dev-utils/launchEditor');
92+
launchPowerShellAgent();
93+
}
8894
});
8995

9096
['SIGINT', 'SIGTERM'].forEach(function(sig) {

0 commit comments

Comments
 (0)