Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking: Drop browser helpers from cli-tools, simplify middleware #2588

Merged
merged 1 commit into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/cli-server-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
},
"dependencies": {
"@react-native-community/cli-tools": "16.0.2",
"body-parser": "^1.20.3",
"compression": "^1.7.1",
"connect": "^3.6.5",
"errorhandler": "^1.5.1",
"nocache": "^3.0.1",
"open": "^6.2.0",
"pretty-format": "^26.6.2",
"serve-static": "^1.13.1",
"ws": "^6.2.3"
Expand Down
5 changes: 3 additions & 2 deletions packages/cli-server-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import nocache from 'nocache';
import serveStatic from 'serve-static';

import indexPageMiddleware from './indexPageMiddleware';
import openStackFrameInEditorMiddleware from './openStackFrameInEditorMiddleware';
import openStackFrameMiddleware from './openStackFrameMiddleware';
import openURLMiddleware from './openURLMiddleware';
import rawBodyMiddleware from './rawBodyMiddleware';
import securityHeadersMiddleware from './securityHeadersMiddleware';
Expand Down Expand Up @@ -35,9 +35,10 @@ export function createDevServerMiddleware(options: MiddlewareOptions) {
.use(compression())
.use(nocache())
.use('/', indexPageMiddleware)
.use('/open-stack-frame', openStackFrameInEditorMiddleware(options))
.use('/open-stack-frame', openStackFrameMiddleware(options))
.use('/open-url', openURLMiddleware)
.use('/status', statusPageMiddleware)
// TODO: Remove. Requires standardized JSON body parsing support in Metro.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun 😄. We have a fix to make around this in Metro. See the original CLI PR: #1147.

I’ve actioned an initial change in React Native — also required for the substitution of body-parser for the endpoints in this PR:

Once removed, we can simplify further and apply .use(json()) against this entire middleware stack.

.use('/symbolicate', rawBodyMiddleware)
// @ts-ignore mismatch
.use('/systrace', systraceProfileMiddleware)
Expand Down
35 changes: 0 additions & 35 deletions packages/cli-server-api/src/openStackFrameInEditorMiddleware.ts

This file was deleted.

52 changes: 52 additions & 0 deletions packages/cli-server-api/src/openStackFrameMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import type {IncomingMessage, ServerResponse} from 'http';

import {json} from 'body-parser';
import connect from 'connect';
import {launchEditor} from '@react-native-community/cli-tools';

type Options = {
watchFolders: ReadonlyArray<string>;
Copy link
Collaborator Author

@huntie huntie Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szymonrybczak Relevant to your other comment, watchFolders is staying around at this level because it ought to (not currently) be used to parse out substitutions from URLs sent by the debugger, e.g. '[metro-project]/path/to/file.js'.

cc @hoxyq @robhogan

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, thanks for explanation!

};

/**
* Open a stack frame in the user's text editor.
*/
export default function openStackFrameMiddleware(_: Options) {
const handler = (
req: IncomingMessage & {
// Populated by body-parser
body?: Object;
},
res: ServerResponse,
next: (err?: Error) => void,
) => {
if (req.method === 'POST') {
if (req.body == null) {
res.writeHead(400);
res.end('Missing request body');
return;
}

const frame = req.body as {
file: string;
lineNumber: number;
};

launchEditor(frame.file, frame.lineNumber);

res.writeHead(200);
res.end();
}

next();
};

return connect().use(json()).use(handler);
}
44 changes: 29 additions & 15 deletions packages/cli-server-api/src/openURLMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,40 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import http from 'http';
import {launchDefaultBrowser, logger} from '@react-native-community/cli-tools';

import type {IncomingMessage, ServerResponse} from 'http';

import {json} from 'body-parser';
import connect from 'connect';
import rawBodyMiddleware from './rawBodyMiddleware';
import open from 'open';

/**
* Handle request from JS to open an arbitrary URL in Chrome
* Open a URL in the system browser.
*/
function openURLMiddleware(
req: http.IncomingMessage & {rawBody?: string},
res: http.ServerResponse,
next: (err?: any) => void,
async function openURLMiddleware(
req: IncomingMessage & {
// Populated by body-parser
body?: Object;
},
res: ServerResponse,
next: (err?: Error) => void,
) {
if (!req.rawBody) {
return next(new Error('missing request body'));
if (req.method === 'POST') {
if (req.body == null) {
res.writeHead(400);
res.end('Missing request body');
return;
}

const {url} = req.body as {url: string};

await open(url);

res.writeHead(200);
res.end();
}
const {url} = JSON.parse(req.rawBody);
logger.info(`Opening ${url}...`);
launchDefaultBrowser(url);
res.end('OK');

next();
}

export default connect().use(rawBodyMiddleware).use(openURLMiddleware);
export default connect().use(json()).use(openURLMiddleware);
1 change: 0 additions & 1 deletion packages/cli-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"find-up": "^5.0.0",
"launch-editor": "^2.9.1",
"mime": "^2.4.1",
"open": "^6.2.0",
"ora": "^5.4.1",
"prompts": "^2.4.2",
"semver": "^7.5.2"
Expand Down
2 changes: 0 additions & 2 deletions packages/cli-tools/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ export {default as logger} from './logger';
export {default as isPackagerRunning} from './isPackagerRunning';
export {default as getDefaultUserTerminal} from './getDefaultUserTerminal';
export {fetch, fetchToTemp} from './fetch';
export {default as launchDefaultBrowser} from './launchDefaultBrowser';
export {default as launchDebugger} from './launchDebugger';
export {default as launchEditor} from './launchEditor';
export * as version from './releaseChecker';
export {default as resolveNodeModuleDir} from './resolveNodeModuleDir';
Expand Down
16 changes: 0 additions & 16 deletions packages/cli-tools/src/launchDebugger.ts

This file was deleted.

26 changes: 0 additions & 26 deletions packages/cli-tools/src/launchDefaultBrowser.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/cli-tools/src/launchEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import launchEditorImpl from 'launch-editor';
export default function launchEditor(
fileName: string,
lineNumber: number,
_watchFolders: ReadonlyArray<string>,
_watchFolders?: ReadonlyArray<string>,
): void {
launchEditorImpl(`${fileName}:${lineNumber}`, process.env.REACT_EDITOR);
}
18 changes: 0 additions & 18 deletions packages/cli-tools/src/throwIfNonAllowedProtocol.ts

This file was deleted.

Loading
Loading