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

fix(cli-service): pass --public host to devserver #6066

Merged
merged 2 commits into from
Nov 20, 2020

Conversation

jonaskuske
Copy link
Contributor

@jonaskuske jonaskuske commented Nov 18, 2020

close #3220

Prevent "Invalid Host" error by passing public host through to webpack-dev-server when --public is used

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

When you use the public option in webpack-dev-server it'll automatically allow requests from the specified host, without needing the additional allowedHosts[] or disableHostCheck options. However when the --public option in Vue CLI was used, the value was not passed through to webpack-dev-server (as HMR client injection is handled separately) so you still got the Invalid host error in that case.

This PR extracts the host from the --public arg and passes it through to the dev server to prevent this error.

close vuejs#3220

Prevent "Invalid Host" error by passing public host to dev server when --public is used
@jonaskuske
Copy link
Contributor Author

jonaskuske commented Nov 19, 2020

Done as suggested, --public overrides devServer.public now. :)


However, note that devServer.public isn't 100% equivalent to --public atm.
Even though the DevServer docs unfortunately don't really mention this, devServer.public needs to be a host, not a URL – it'll strip the port when it determines the hostname to allow, but not the protocol etc.

At the same time, --public is documented as "specify the public network URL for the HMR client" – which makes sense, as Vue CLI also uses it as the page to --open, so the protocol might be necessary if you want Vue CLI to correctly open the https version in your browser. (DevServer accepts an additional option called openPage for this purpose, but of course Vue CLI force-disables webpack's built-in open functionality entirely)

So, long story short:

  1. --public http://myhost.test will work because Vue CLI accepts URLs and will determine the host, but devServer.public: 'http://myhost.test' will still give you an Invalid Host error

  2. Vue CLI does not allow you to configure the HMR network URL separately from the URL you want to open on launch, e.g. if you run your site through my-host.test but your server doesn't handle websockets so you still want the HMR client to connect directly to 127.0.0.1:8080 – is this worth opening an issue?

@fangbinwei
Copy link
Collaborator

--public http://myhost.test will work because Vue CLI accepts URLs and will determine the host, but devServer.public: 'http://myhost.test' will still give you an Invalid Host error

Your codes fix this problem, so we can handle devServer.public: 'http://myhost.test' . Since the priority of publicHost is higher than devServer.public now.

When you use the public option in webpack-dev-server it'll automatically allow requests from the specified host, without needing the additional allowedHosts[] or disableHostCheck options

Your codes make CLI behave the same as webpack-dev-server in this respect.

Vue CLI does not allow you to configure the HMR network URL separately from the URL you want to open on launch,

I think it's a very corner case.

Moreover, there may be some historical reasons that I don’t know make CLI and webpack-dev-server behave inconsistently

@jonaskuske
Copy link
Contributor Author

Your codes make CLI behave the same as webpack-dev-server in this respect.

Ah, you're right of course, somehow totally missed that my code that extracts the host will be run both for --public and the dev server option. 😅

Well, guess this PR is ready then?

Copy link
Collaborator

@fangbinwei fangbinwei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fangbinwei fangbinwei merged commit 33f954c into vuejs:dev Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set allowedHosts option when serving with --public option
3 participants