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: don't default to localhost in production #72

Closed
wants to merge 3 commits into from

Conversation

galvez
Copy link

@galvez galvez commented Sep 7, 2019

Noticed a live deployment with @nuxt/http was using localhost if no environment variable was set.

@galvez galvez requested a review from pi0 September 7, 2019 17:24
@codecov-io
Copy link

Codecov Report

Merging #72 into dev will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev      #72   +/-   ##
=======================================
  Coverage   97.05%   97.05%           
=======================================
  Files           1        1           
  Lines          34       34           
  Branches       16       16           
=======================================
  Hits           33       33           
  Misses          1        1
Impacted Files Coverage Δ
lib/module.js 97.05% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1674aae...e4ade66. Read the comment docs.

@galvez
Copy link
Author

galvez commented Sep 7, 2019

@pi0 erm, still needs a fix -- wait :)

@galvez
Copy link
Author

galvez commented Sep 7, 2019

K, fixed it.

@galvez
Copy link
Author

galvez commented Sep 7, 2019

Related: nuxt/press#58 -- actually @pi0 would appreciate if you could double check and make sure this is really the issue.

@pi0
Copy link
Member

pi0 commented Sep 9, 2019

0.0.0.0 is not a valid IP. Maybe we can use this.options.server.host which handles --host as well.

See related PR for axios module: nuxt-community/axios-module#245

@pi0
Copy link
Member

pi0 commented Nov 2, 2019

/reminder @galvez What do you think about proposed change?

@galvez
Copy link
Author

galvez commented Nov 5, 2019

Let's do that :)

@pi0
Copy link
Member

pi0 commented Jan 30, 2020

Suggestion applied by bc58738. Implementation is now consistent with axios module to support CLI flags or nuxt config if provided.

@pi0 pi0 closed this Jan 30, 2020
@pi0 pi0 deleted the fix/no-localhost-in-production branch January 30, 2020 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants