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

refactor: use the cli-server-api package #1140

Merged
merged 3 commits into from
May 8, 2020

Conversation

fson
Copy link
Contributor

@fson fson commented Apr 27, 2020

Summary:

The development server middleware and WebSocket servers were extracted out to the @react-native-community/cli-server-api package in #1118. This PR makes the CLI use that package and removes any remaining duplicates of these modules that have been moved.

Test Plan:

Followed the Testing your changes instructions in CONTRIBUTING.md and started the server with npx react-native start --watchFolders /Users/ville/Projects/react-native-cli/ --resetCache. Verified the following features work:

  • Running the app on iOS simulator
  • Editing a file in the app, the app refreshed automatically
  • Refreshing the app by pressing r
  • Starting the debugger in Chrome and setting a breakpoint
  • Looking at logs, inspecting elements and refreshing the app in Flipper

@@ -9,13 +9,13 @@
import Metro from 'metro';
Copy link
Member

Choose a reason for hiding this comment

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

Can we also rename the directory from server to start now that most of the implementation moved to other package? I think it's a good moment to make it happen and be consistent with other commands

Copy link
Contributor Author

@fson fson Apr 28, 2020

Choose a reason for hiding this comment

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

Sure, renamed the directory to commands/start.

@thymikee thymikee force-pushed the use-dev-server-api branch from 8bbe167 to d768303 Compare May 8, 2020 08:09
@thymikee
Copy link
Member

thymikee commented May 8, 2020

Noticed an issue with "Reload app" button in Chrome (works fine with Flipper), opening another debugger tab unnecessarily. Same when pressing Reload from an app. Tested on Android

@@ -79,6 +79,7 @@ export function createDevServerMiddleware(options: MiddlewareOptions) {
'/message',
);
broadcast = messageSocket.broadcast;
isDebuggerConnected = debuggerProxy.isDebuggerConnected;
Copy link
Member

Choose a reason for hiding this comment

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

cc @fson the debugger connected state was not updated, likely an oversight when moving code around

@thymikee thymikee merged commit 2c08a82 into react-native-community:master May 8, 2020
@fson fson deleted the use-dev-server-api branch May 8, 2020 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants