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

v6.0.1: Fix URL the browser opens when the Dev Server is being proxied #54

Merged
merged 2 commits into from Sep 9, 2018

Conversation

homer0
Copy link
Owner

@homer0 homer0 commented Sep 9, 2018

What does this PR do?

This issue was caused by #35. When I implemented the custom plugin for handling logging and opening the browser, I completely ignored the proxied setting, so if the dev server was being proxied, the browser was still opening the base URL.

This hotfix fixes the issue: It adds a new url setting to the proxied object _normalizeTargetDevServerSettings returns. This URL can change depending on 2 things:

  1. If proxied.enabled is true but no host is specified, the url will copy the one built for the dev server as if is not being proxied.
  2. If proxied.enabled is true an a host was specified, the url will be built using that host.

How should it be tested manually?

You can test it with a configuration like this one:

{
  ...
  devServer: {
    proxied: {
      enabled: true,
      host: 'my-local-domain.com',
      https: true,
    },
    port: 2525,
  },
}

When you run your target for development, instead of opening http://localhost:2525, it should open https://my-local-domain.com (Up to you to setup my-local-domain on your computer with SSL enabled :P).

And of course...

npm test
# or
yarn test

@coveralls
Copy link

Pull Request Test Coverage Report for Build 194

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 191: 0.0%
Covered Lines: 574
Relevant Lines: 574

💛 - Coveralls

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.

None yet

2 participants