-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Code clean-ups and createServer migration
Summary: Scope of the diff: 1. Middleware `react-native-github/local-cli` and `react-native-internal-cli` uses a very similar set of middlewares (internal cli extends github version), so I decided to move it to a standalone file (middleware manager) in order to remove duplications and increase readability. 2. Types Seems that after Flow upgrade to version 0.68 there were many type issues to resolve, so all of them were auto-mocked. This is fine, but I'd like to see Flow assists me with `Metro.createServer` -> `Metro.runServer` migration. Hence, I decided to resolve flow mocks, related to runServer. 3. `runServer` signature In `react-native-github` repo I cleaned up `runServer` signature by removing `startCallback` and `readyCallback` from the function parameters and moved them to `runServer` instead. 4. Replace `createServer` by `runServer` In `react-native-github` repo, `createServer` has been replaced by `runServer`. __Some of arguments are not mapped__. Note that this diff will partially break argument mapping. This is intentional. @[100000044482482:ives] will fix it with a new config package. Reviewed By: mjesun Differential Revision: D8711717 fbshipit-source-id: a843ab576360ff7242099910d8f25a9cb0a388c0
- Loading branch information
1 parent
ee535fa
commit c4a66a8
Showing
4 changed files
with
113 additions
and
179 deletions.
There are no files selected for viewing
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,70 @@ | ||
/** | ||
* Copyright (c) 2013-present, Facebook, Inc. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @strict | ||
* @flow | ||
*/ | ||
|
||
const compression = require('compression'); | ||
const connect = require('connect'); | ||
const errorhandler = require('errorhandler'); | ||
const morgan = require('morgan'); | ||
const path = require('path'); | ||
const serveStatic = require('serve-static'); | ||
const WebSocketServer = require('ws').Server; | ||
|
||
const indexPageMiddleware = require('./indexPage'); | ||
const copyToClipBoardMiddleware = require('./copyToClipBoardMiddleware'); | ||
const loadRawBodyMiddleware = require('./loadRawBodyMiddleware'); | ||
const openStackFrameInEditorMiddleware = require('./openStackFrameInEditorMiddleware'); | ||
const statusPageMiddleware = require('./statusPageMiddleware.js'); | ||
const systraceProfileMiddleware = require('./systraceProfileMiddleware.js'); | ||
const getDevToolsMiddleware = require('./getDevToolsMiddleware'); | ||
|
||
type Options = { | ||
+watchFolders: $ReadOnlyArray<string>, | ||
+host?: string, | ||
} | ||
|
||
type WebSocketProxy = { | ||
server: WebSocketServer, | ||
isChromeConnected: () => boolean, | ||
}; | ||
type Connect = any; | ||
|
||
module.exports = class MiddlewareManager { | ||
app: Connect; | ||
options: Options; | ||
|
||
constructor(options: Options) { | ||
const debuggerUIFolder = path.join(__dirname, 'util', 'debugger-ui'); | ||
|
||
this.options = options; | ||
this.app = connect() | ||
.use(loadRawBodyMiddleware) | ||
.use(compression()) | ||
.use('/debugger-ui', serveStatic(debuggerUIFolder)) | ||
.use(openStackFrameInEditorMiddleware(this.options)) | ||
.use(copyToClipBoardMiddleware) | ||
.use(statusPageMiddleware) | ||
.use(systraceProfileMiddleware) | ||
.use(indexPageMiddleware) | ||
.use(morgan('combined')) | ||
.use(errorhandler()); | ||
} | ||
|
||
serveStatic = (folder: string) => { | ||
this.app.use(serveStatic(folder)); | ||
}; | ||
|
||
getConnectInstance = () => this.app; | ||
|
||
attachDevToolsSocket = (socket: WebSocketProxy) => { | ||
this.app.use( | ||
getDevToolsMiddleware(this.options, () => socket.isChromeConnected()), | ||
); | ||
}; | ||
}; |
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
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
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
2 comments
on commit c4a66a8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kureev This seems to be causing a few issues for me, looks like we're not passing assetRegistryPath anymore which causes an error. Also it looks like we don't add the middlewares anymore, or I don't understand how. middlewareManager
isn't passed to the http server started by metro.
I'm currently investigating CI failure and it fails to detect that the packager started because the /status endpoint doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I think I fixed it 🙃 #20162
The
initialize_done
andinitialize_failed
events don't seem to get triggered anymore after this change. I thinkrunServer
needs to be changed to triggerinitialize_done
.(I was looking into using
initialize_done
to detect when the HTTP server is ready.)