Skip to content

Commit

Permalink
Fix port as -1 if dev server without specifying port on Android (face…
Browse files Browse the repository at this point in the history
…book#34705)

Summary:
when specifying dev server without port, e.g. http://www.example.com/, there are some issues.

1. redbox error
<img src="https://user-images.githubusercontent.com/46429/190540390-8ee420f2-7642-427b-9f2e-e0c6d31015f8.png" width="30%">

2. showing -1 in loading view

<img src="https://user-images.githubusercontent.com/46429/190540727-158f35ad-359f-443a-a4b0-768dd2f7e400.png" width="50%">

the root cause is coming from [`java.net.URL.getPort()` will return -1 when the url doesn't have a port](https://developer.android.com/reference/java/net/URL#getPort()). this pr replaces the parser to [`okhttp3.HttpUrl`](https://square.github.io/okhttp/4.x/okhttp/okhttp3/-http-url/#port) that it will have default port 80 for http or port 443 for https. the two call paths should only serve http/https address, not file:// address. it should be safe to change from java.net.URL to okhttp3.HttpUrl.

not fully related, in the case above, android will connect to `ws://www.example.com/:8097` for react-devtools
we should strip the trailing slash in *setUpReactDevTools.js*

## Changelog

[Android] [Fixed] - Fix port as -1 if dev server without specifying port on Android

Pull Request resolved: facebook#34705

Test Plan:
test on rn-tester with the following steps

1. `yarn start`
2. open another terminal and run `ngrok http 8081` and it will return a tunnel url, e.g. `71a1-114-36-194-97.jp.ngrok.io`
3. open dev setting in app and change the dev server to `71a1-114-36-194-97.jp.ngrok.io`
5. reload the app

Reviewed By: cipolleschi

Differential Revision: D39573988

Pulled By: cortinico

fbshipit-source-id: 397df90ab30533207bd87a3f069132d97c22c7fd
  • Loading branch information
Kudo authored and OlimpiaZurek committed May 22, 2023
1 parent 08af34a commit 48c9d82
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
5 changes: 4 additions & 1 deletion Libraries/Core/setUpReactDevTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ if (__DEV__) {
// Get hostname from development server (packager)
const devServer = getDevServer();
const host = devServer.bundleLoadedFromServer
? devServer.url.replace(/https?:\/\//, '').split(':')[0]
? devServer.url
.replace(/https?:\/\//, '')
.replace(/\/$/, '')
.split(':')[0]
: 'localhost';

// Read the optional global variable for backward compatibility.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public void showForUrl(String url) {
return;
}

int port = parsedURL.getPort() != -1 ? parsedURL.getPort() : parsedURL.getDefaultPort();
showMessage(
context.getString(
R.string.catalyst_loading_from_url, parsedURL.getHost() + ":" + parsedURL.getPort()));
context.getString(R.string.catalyst_loading_from_url, parsedURL.getHost() + ":" + port));
}

public void showForRemoteJSEnabled() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ private void resetCurrentContext(@Nullable ReactContext reactContext) {
URL sourceUrl = new URL(getSourceUrl());
String path = sourceUrl.getPath().substring(1); // strip initial slash in path
String host = sourceUrl.getHost();
int port = sourceUrl.getPort();
int port = sourceUrl.getPort() != -1 ? sourceUrl.getPort() : sourceUrl.getDefaultPort();
mCurrentContext
.getJSModule(HMRClient.class)
.setup("android", path, host, port, mDevSettings.isHotModuleReplacementEnabled());
Expand Down

0 comments on commit 48c9d82

Please sign in to comment.