This repository has been archived by the owner on Dec 13, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 682
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Summary:This greatly simplifies the RN node executor. Previously, messages from the RN app would be translated by `ChildManager` into calls of `Child` methods, which would then create its own message format to send to the executor. Responsibilities for handling these messages were divided between all three. After this change, the RN message format is maintained all the way to executor. Not only does this remove unnecessary transformations, it also makes things a lot easier to understand for people familiar with RN already. In addition, I've moved the script loading into the executor. This results in a clearer division of responsibilities, with RN sending and receiving messages to the executor and Nuclide only being there to facilitate the transfer. With that in mind, I was able to replace `Child` with a function (`executeRnRequests`) that accepts a stream of RN messages and outputs a stream of results. Since this sums up all of Nuclide's responsibilities here, I'd like to remove `ChildManager` entirely (it's now just an OO interface to `executeRnRequests`) but it's currently being used by the `ExecutorServer`, so I'll hold off until we're able to remove that in favor of the new `DebuggerProxyClient`. This refactor also allowed me to switch from calling `process._debugProcess` on a running process to starting it with `--debug-brk` which fixed the issue with the debugger not skipping the loader breakpoint when using node 5.x (since "restartframe" is no longer called?see nodejs/node#5221). public Reviewed By: ssorallen Differential Revision: D3073832 fb-gh-sync-id: 3604c2d19c5ab336b854766ff10c5a77695bade5 shipit-source-id: 3604c2d19c5ab336b854766ff10c5a77695bade5
- Loading branch information
1 parent
58e8778
commit 152cbe0
Showing
6 changed files
with
223 additions
and
313 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
80 changes: 80 additions & 0 deletions
80
pkg/nuclide-react-native-node-executor/lib/executeRnRequests.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
'use babel'; | ||
/* @flow */ | ||
|
||
/* | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the license found in the LICENSE file in | ||
* the root directory of this source tree. | ||
*/ | ||
|
||
import type {ExecutorResponse, RnRequest} from './types'; | ||
|
||
import featureConfig from '../../nuclide-feature-config'; | ||
import { | ||
createProcessStream, | ||
forkWithExecEnvironment, | ||
getOutputStream, | ||
} from '../../nuclide-commons/lib/process'; | ||
import {getLogger} from '../../nuclide-logging'; | ||
import {CompositeDisposable} from 'atom'; | ||
import path from 'path'; | ||
import {Observable} from 'rx'; | ||
|
||
const logger = getLogger(); | ||
|
||
export function executeRnRequests(rnRequests: Observable<RnRequest>): Observable<ExecutorResponse> { | ||
const workerProcess = createProcessStream(() => ( | ||
// TODO: The node location/path needs to be more configurable. We need to figure out a way to | ||
// handle this across the board. | ||
forkWithExecEnvironment( | ||
path.join(__dirname, 'executor.js'), | ||
[], | ||
{ | ||
execArgv: ['--debug-brk'], | ||
execPath: featureConfig.get('nuclide-react-native.pathToNode'), | ||
silent: true, | ||
}, | ||
) | ||
)); | ||
|
||
return Observable.merge( | ||
workerProcess.map(process => ({ | ||
kind: 'pid', | ||
pid: process.pid, | ||
})), | ||
|
||
// The messages we're receiving from the worker process. | ||
( | ||
workerProcess.flatMap( | ||
process => Observable.fromEvent(process, 'message') | ||
): Observable<ExecutorResponse> | ||
), | ||
|
||
Observable.create(() => ( | ||
new CompositeDisposable( | ||
// Send the incoming requests to the worker process for evaluation. | ||
rnRequests | ||
.withLatestFrom(workerProcess, (r, p) => ([r, p])) | ||
.subscribe(([request, process]) => { process.send(request); }), | ||
|
||
// Pipe output from forked process. This just makes things easier to debug for us. | ||
workerProcess | ||
.flatMapLatest(process => getOutputStream(process)) | ||
.subscribe(message => { | ||
switch (message.kind) { | ||
case 'error': | ||
logger.error(message.error.message); | ||
return; | ||
case 'stderr': | ||
case 'stdout': | ||
logger.info(message.data.toString()); | ||
return; | ||
} | ||
}), | ||
) | ||
)), | ||
|
||
).share(); | ||
} |
Oops, something went wrong.