apache / cordova-browser

Apache Cordova
Apache License 2.0
170 stars 85 forks source link

CB-8182 - port 'cordova serve' to 'cordova run browser' #9

Closed kirkshoop closed 9 years ago

kirkshoop commented 9 years ago

This ports the code to serve a project from cordova serve The result is that --disable-web-security is no longer required and all other side-effects of using a real http url instead of a file url can be observed when testing a project. This and #8 will conflict. Nikhil or I will rebase the second PR onto whichever is first to be accepted.

jsoref commented 9 years ago

Note that your commit messages should be of the form: CB-8182 Add ... or CB-8182 Port ... instead of CB-8182 - add ...

Please use backticks around d8 and mime. Please use backticks ('`') instead of single quotes ("'") for commands / identifiers.

jsoref commented 9 years ago

Why can't you just make cordova serve work with cordova-browser/cordova run? I really don't want to have a fork of cordova serve.

kirkshoop commented 9 years ago

rebased the commits to master and changes the messages.

kirkshoop commented 9 years ago

I agree that duplicating functionality has consequences. I would like to understand the scenarios that cordova serve supports. If the existing cordova serve scenarios can be satisfied by cordova platform add browser cordova run browser then perhaps cordova serve could be deprecated and removed. If they actually support different scenarios, then I think that both will need to exist.

kirkshoop commented 9 years ago

@jsoref Thanks for the comments, please see my updates

jsoref commented 9 years ago

cordova serve is potentially usable by cordova-android or cordova-ios or cordova-blackberry, so, no, please don't just fork it or hand wave and pray that forcing people to use cordova-browser will somehow work.

jsoref commented 9 years ago

Specifically, it in theory allows one to live(-ish) edit an application instead of constantly republishing it to the device.

kirkshoop commented 9 years ago

no prayer was involved.

As I said, if there are good scenarios for cordova serve then it should continue to exist. You have explained that it should exist and I agree with you.

The scenarios for cordova run browser are different and require that the project be hosted over http in the browser platform as well. Currently, a file url is used which causes issues with CORS headers and other effects that cause cordova run browser with a file url to behave completely differently than it will when actually hosted as a web site. These differences need to be minimized to ensure that more bugs can be reproduced and fixed locally.

kirkshoop commented 9 years ago

@jsoref do you have a proposal for how cordova run browser should be structured?

initial options:

purplecabbage commented 9 years ago

Ideally, I think 'cordova serve' should become it's own module in npm (cordova-serve) which is used both by cordova-lib and by cordova-browser.

jsoref commented 9 years ago

fwiw, that was what i was going to suggest. :+1:

purplecabbage commented 9 years ago

It's worth a lot! There is some prior art on this, last summer Suraj, aka @surajpindoria split all of cordova-lib into separate modules, however this was too big of a change all at once, so it ultimately became un-mergable rather quickly. It still lives as a parable for future generations here : https://github.com/apache/cordova-lib/pull/62

kirkshoop commented 9 years ago

@nikhilkh I rebased on your changes in #8 Please take a look to see if it is ok.

axemclion commented 9 years ago

Can we make Cordova Serve its own module in a separate commit? I think this commit is critical since we currently serve the browser platform from local file system. The local file system has a different security model and restrictions.

I also think that we should deprecate cordova serve and instead look at how we can make cordova browser enable a similar scenario. Multiple members in the community have said that cordova-browser would have 2 purposes - as a production environment, and as a debug environment. Cordova server should be retired and the cordova browser debug environment should serve that purpose.

@purplecabbage @jsoref - what do you guys think ? Can we merge this, and then look at the bigger question?

axemclion commented 9 years ago

@purplecabbage @jsoref Ping. Wanted your opinions on the question above. I was thinking about the relevance of Cordova serve, given that we have cordova browser run. I also think that we could merge this change, so that we get rid of the issues with serving pages locally. We can then look at re-factoring the code into cordova serve, if we decide to keep it in. Does that make sense ?

purplecabbage commented 9 years ago

Yeah, I think it best to merge this now, and refactor out to a common module later. Please rebase.

TimBarham commented 9 years ago

I'll take care of rebasing. Thanks.

TimBarham commented 9 years ago

Grrr... I merged instead of rebasing. Are we ok with that?

TimBarham commented 9 years ago

Ok, I've cleaned it up. It's ready to merge now.