You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR removes the default --port value for --live-reload. When adding the --https, we already opted out of having a default, and with this (breaking) change there's consistency.
With this PR, if no --port nor --http is specified, the url will be like so http://<host>; in other words, will run in the default port for http (80).
An alternative to this is to merge --host, --port, and --https CLI args into a single --url or --live-reload-url arg, that would directly override server.url config. I am inclined to open a separate PR for that and the team can decide which one is preferable.
Change Type
Fix
Feature
Refactor
Breaking Change
Documentation
Rationale / Problems Fixed
Port '3000' isn't widely used for local servers, maybe it made sense once upon a time, but not anymore.
There has been confusion regarding the use of a default port for --live-reload (there's probably older links, but these should suffice to justify the change):
Since the port may change depending on what's the underlying stack used to setup live-reload, the framework shouldn't make assumptions on the port to use.
Tests or Reproductions
You can use any existing capacitor app to test, you just need to point to the local capacitor cli version from this PR's branch.
I personally prefer a single --url param rather than having to pass 4 different params: --live-reload, --host--port and --https, with undocumented defaults if you don't pass some of them.
When you run a dev server and/or ngrok they usually tell you, the app is running on http://192.168.1.53:3713/https://4f8a-83-53-7-217.ngrok-free.app or similar message, so with --url you could simply copy the whole url and paste as the --url param, while with current implementation you have to pass --live-reload --host 192.168.1.53 --port 3713, or if using ngrok not pass the --port, but pass --https.
Also I find --live-reload param very confusing, we just changed the description recently to try to make it cleared, but still the param name is confusing as it doesn't really allow live reload, it just configures the server.url, whether it points to a server that supports live reload or not, and doesn't do any "live reload magic" by itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the default
--portvalue for--live-reload. When adding the --https, we already opted out of having a default, and with this (breaking) change there's consistency.With this PR, if no
--portnor--httpis specified, the url will be like sohttp://<host>; in other words, will run in the default port for http (80).An alternative to this is to merge
--host,--port, and--httpsCLI args into a single--urlor--live-reload-urlarg, that would directly overrideserver.urlconfig. I am inclined to open a separate PR for that and the team can decide which one is preferable.Change Type
Rationale / Problems Fixed
Port '3000' isn't widely used for local servers, maybe it made sense once upon a time, but not anymore.
There has been confusion regarding the use of a default port for
--live-reload(there's probably older links, but these should suffice to justify the change):cap runignores server config #8303Since the port may change depending on what's the underlying stack used to setup live-reload, the framework shouldn't make assumptions on the port to use.
Tests or Reproductions
You can use any existing capacitor app to test, you just need to point to the local capacitor cli version from this PR's branch.
For reference, I used https://github.com/OS-pedrogustavobilro/capacitor-test-livereload
Screenshots / Media
Not relevant
Platforms Affected
Notes / Comments
N/A