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

devServer.js config with environment variable #228

Closed
pdavidow opened this issue Jul 13, 2016 · 15 comments
Closed

devServer.js config with environment variable #228

pdavidow opened this issue Jul 13, 2016 · 15 comments

Comments

@pdavidow
Copy link
Contributor

pdavidow commented Jul 13, 2016

In devServer.js, instead of

app.listen(3000, 'localhost', function (err) {

for Cloud9 this should be:

app.listen(8080, '0.0.0.0', function(err) { 

and Nitrous.io:

app.listen(3000, '0.0.0.0', function (err) {

This could be configured with an environment variable.

The console.log output of course would also need to be updated, and for Cloud9 should also mention that Open in Preview should not be used.

@ericelliott
Copy link
Owner

Would you like to file a pull request? =)

I don't think we need to specifically mention Cloud9. The environment variables should be enough to allow users to make it work on Cloud9.

@pdavidow
Copy link
Contributor Author

pdavidow commented Jul 14, 2016

Perhaps another approach is a flag on npm start.
Environment Variables are only for Windows (I think), but what if someone wants to run Cloud9 say from a Chromebook? Plus, even on Windows, someone may have an account both on Cloud9 and Nitrous etc.

@ericelliott
Copy link
Owner

Environment variables work great on any platform, but the way you set them varies depending on platform. However, we should not care about how it gets set, and only respect the environment variables if they are set.

So, for example, you could create host and port variables like this:

const {
  host = 'localhost',
  port = 3000
} = process.env;

Now in your cloud9 console, you can set the environment variable before launching the service:

set HOST='0.0.0.0'
set PORT=8080
npm start

@pdavidow
Copy link
Contributor Author

pdavidow commented Jul 14, 2016

There doesn't seem to be anything to test, so don't know how to write test for this.

Also, since this codebase is written in ES5, new code I would think should conform with something like:

var host = process.env.HOST;
var port = process.env.PORT;

host = host ? host : 'localhost';
port = port ? port : 3000;

@pdavidow
Copy link
Contributor Author

I am stuck on manual testing: Unable to set HOST in bash shell.

@ericelliott
Copy link
Owner

  1. This codebase is written in ES6.
  2. If HOST is reserved, I'm happy using something less generic like PC_HOST and PC_PORT

@pdavidow
Copy link
Contributor Author

When I try to send Pull Request, I get Authentication Failed.

@ericelliott
Copy link
Owner

@pdavidow Have you set up your GitHub SSH Keys?

@pdavidow
Copy link
Contributor Author

pdavidow commented Jul 22, 2016

The SSH keys are all set, but it still doesn't help.
Instead, I have pushed branch here: https://github.com/pdavidow/react-pure-component-starter

(Tested manually on Cloud9, and Mac OSX)

@ericelliott
Copy link
Owner

If you delete that and just hit the fork button on this repo, then pull your fork and merge your changes into that, then push your changes to your fork, you should be able to submit a pull request. You should see a yellow button pop up asking if you want to submit a PR.

@ericelliott
Copy link
Owner

See Forking a repo

@pdavidow
Copy link
Contributor Author

pdavidow commented Jul 22, 2016

The deed has been done, I think. A yellow button (actually, a yellow ring) did pop up, but all it's doing is notifying me of pending CI tests -- it's not asking me to submit a PR. (Anyway, the reason it didn't originally work is because I had cloned without making a fork.)

@ericelliott
Copy link
Owner

Nice.. I'll look over it asap

@pdavidow
Copy link
Contributor Author

How would you write tests for this?

@ericelliott
Copy link
Owner

Set the environment variable. Launch the server. Use something like supertest to make sure the server responds appropriately to requests on the specified host/port.

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

No branches or pull requests

2 participants