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

get server ip automatically #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

janpio
Copy link
Member

@janpio janpio commented Nov 24, 2018

Get the local server ip automatically instead of from command line

by @johanlantz

@project-bot project-bot bot added this to 🐣 New PR / Untriaged in Apache Cordova: Tooling Pull Requests Nov 24, 2018
@janpio
Copy link
Member Author

janpio commented Nov 28, 2018

While I like the general idea of this, I think the exact implementation is problematic @johanlantz. Currently this overwrites the logic in

LocalServer.prototype.getConnectionAddress = function (platformId) {
if (this.externalServerUrl) {
return this.externalServerUrl;
}
// build connection uri for localhost based on platform
var connectionUrl;
switch(platformId) {
case "android" :
connectionUrl = "http://10.0.2.2";
break;
case "ios" :
case "browser" :
case "windows" :
/* falls through */
default:
connectionUrl = "http://127.0.0.1";
}
return connectionUrl;
};
for Android (although as the comment here, I am not sure this is really a good idea anyway).

Probably the whole "get IP for server" code has to be looked at to solve this in a proper way.

Until then, a nice workaround seems to be to use the --useTunnel option so it just loops through the internet. Not efficient, but works.

@johanlantz
Copy link
Contributor

at your discression, I just did a solution that solved my immediate problem. Much better if you take a whole approach to not cause problems in other places

@purplecabbage
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants