phonegap / phonegap-roadmap

Upcoming milestones and projects for PhoneGap
4 stars 6 forks source link

Moving phonegap serve into cordova serve #217

Open timkim opened 6 years ago

timkim commented 6 years ago

Hello all,

As you may or may not know, the cordova serve command and the phonegap serve command were roughly created at the same time some years ago to help users preview their work on a browser or on device. Once entering the command, a server in node was spun up to serve the Cordova/PhoneGap's project www/ to the requesting client.

cordova serve if I remember correctly was mainly authored by the Googlers with the intention of being more focused for their Chrome Web Apps stuff. Whereas phonegap serve was meant to be consumed by the PhoneGap Developer App. However, projects being served by cordova serve could not be consumed by the PhoneGap Developer App.

I don't quite remember what happened to the cordova serve after the Googlers left but it seems like it was touched by the Microsoft people for a little bit. However, I'm kinda guessing it's not a very used command within the cordova cli.

With some movement on the PhoneGap cli side, in that the PhoneGap cli is going to be re-organised, it seems like the phonegap serve command could find a new home by replacing the current cordova serve.

So what I'm proposing is:

I have some test branches with some work done in them already. I've gotten as far as getting the phonegap serve command working but it lacks polish because I sorta just jammed everything in there and stopped when it could serve a project. https://github.com/timkim/cordova-serve/commits/master https://github.com/timkim/cordova-lib

Benefits for Cordova: 1) The way cordova serve serves assets doesn't allow CORS or other requests in your app to go through without a lot of hacks. Since the resources are served by the server, its origin is set to that vs if it were an app, the origin would be the file uri. phonegap serve solves this by using the content-sync plugin so resources are back on the file uri as their origin.

2) connect-phonegap mimics the device more closely on browser. In that, when testing within the browser and if you make a plugin call, we have support so that these plugin calls can error out or have mock data.

3) connect-phonegap is better maintained and has been in constant use since 2014.

timkim commented 6 years ago

Oh, on point 2 - it appears cordova serve also mimics plugin calls.

stevengill commented 6 years ago

Some thoughts

1) I like the idea of cordova serve working with phonegap developer app. It makes sense to have this from our teams perspective. Especially if we want to push cordova as local development and phonegap cloud as phonegap workflow.

PhoneGap CLI is not slated for any active development in the near future. If we move this functionality into cordova, it is another case for us potentially sunsetting phonegap cli (not saying that will happen though). With PG Desktop also heading the sunset route, cordova serve may be our only way to do local serve with dev app.

I say we approach this as "cordova serve offering a standard way to serve to companion apps." When we eventually take this to cordova-discuss, we probably want to take the heavy phonegap focus out of the proposal.

I think we should think about the pros and cons to using connect-phonegap approach vs server appraoch. Tim, could you take a stab at that? Discuss the cors issues, CSP injection, etc

Some questions: 1) PhoneGap serve uses gaze to watch your local folder and send updates to dev app when files change. I think this is a valuable workflow for developers. Right now that lives in connect-phonegap. Is there a better way to do this? Does cordova-serve do something similar to this now?

2) phonegap serve auto adds browser. What are the reasons for this and is this something we want to do?

Cordova is currently trying to shed code to make maintenance easier. I feel like this approach would be the opposite of that goal. But it is still worth it IMO.

devgeeks commented 6 years ago

If we are looking at this, could this be the time to look at other serve workflows other than static files and file-watching? I am thinking specifically of webpack dev server and hot module reloading. If cordova serve Could somehow support this (not pretending I know how), that would be amazing.

purplecabbage commented 6 years ago

Personally I think cordova-serve already does too much, it should simply serve files and open the default browser. Currently it is querying the registry in windows to find if Edge browser is installed, and launching Chrome with --user-data-dir flags, which means the user gets a second instance of Chrome running, ....

Adding file-watching for hot reloads, csp injection, multi-touch handling to reload, socket.io logging, ... are all way out of scope in my mind.

Also, connect-phonegap is middleware, cordova-serve is not. imho connect-phonegap should be split into multiple middleware modules that can be endlessly remixed, currently it is way too specific to the dev app.

timkim commented 6 years ago

connect-phonegap

Pros:

Cons:

cordova serve

Pros:

Cons:

timkim commented 6 years ago

Also, for cordova serve to act as a simple http file static server has its merits but I think a large majority of our users make some sort of call to a service. We've tried making a proxy to handle these requests in the early days of connect-phonegap but it was pretty hacky. Real no guarantee to handle every request with a proxy.

I agree though that connect-phonegap could be refactored so it can be more easily composable. It really its own server instance at this point.

macdonst commented 6 years ago

@timkim if we do merge phonegp serve into cordova serve then we should take out the push middleware component as it is tightly tied to phonegap CLI. However, if we can tie this to a release of the PG Dev App with push plugin v2 we won't need the middleware anymore. The middleware is only required to swap the sender ID but that is controlled by a config file instead.

Also, the proxy component works well for xhr but it doesn't take into account the fetch API. We should probably fix that if we are going to tout proxy as a feature of this merge.