fix(cli)!: Remove default port for --live-reload#8376
Open
OS-pedrogustavobilro wants to merge 1 commit intonextfrom
Open
fix(cli)!: Remove default port for --live-reload#8376OS-pedrogustavobilro wants to merge 1 commit intonextfrom
OS-pedrogustavobilro wants to merge 1 commit intonextfrom
Conversation
OS-pedrogustavobilro
requested review from
ItsChaceD,
OS-ruimoreiramendes,
alexgerardojacinto,
andredestro,
eric-horodyski,
jcesarmobile,
markemer and
theproducer
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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