Fix server options to respect port specficied in arch-cli's serve

[chrisvfritz]

This fixes this issue on arch-cli, which really turned out to be an issue here. However, I also discovered that the options passed to the arch server binary are always undefined. I'm not sure if that's expected or not, but thought I'd mention it.

[chrisvfritz]

Once this is in btw, I'm happy to write a quick doc on deploying to Heroku. Everything seems to be working after only minimal configuration. 😃

[charypar]

can we make it process.env.ARCH_PORT || 3000? In case you run the server without the CLI and don't set anything in your environment

[charypar]

Thinking about it a bit more, I'm not sure whether the defaults are the right place to do this (it seems odd that you can affect the defaults by changing env), maybe we should instead pass some arguments in here:

https://github.com/arch-js/arch/blob/master/bin/server.js#L3

@tabazevedo How was this supposed to work? :)

[tabazevedo]

@charypar @chrisvfritz yep, it should be process.env.ARCH_PORT || 3000

I have some stuff mostly working with loading config file using https://github.com/dominictarr/rc which also reads env. Am happy to merge this though (once the default port 3000 is added).

You will be able to pass arguments in as well (you can already, its just a bit clunky and undocumented), so the config priority is server(options) > env > defaults.

[chrisvfritz]

Done!

[charypar]

@tabazevedo are you not bothered by affecting defaults by environment? I admit it's a completely academic debate, but still :) Imagine you use the server module from a different binary, want to keep the port on default but have ARCH_PORT set by mistake or for some unrelated reason... I still feel like we should pass the env var value from the binary itself, rather than the server module.

[chrisvfritz]

I personally don't have a problem with the environment overriding defaults, but I think it would be good to be a little more explicit in the override priority, a la:

default-options = ...
env-options = ...
cli-options = ...
options = ^^default-options import ^^env-options import cli-options

As it is right now, there really is no cli-options passed to the server module (options is always undefined when called from the binary) and the binary sets environment variables in order to indirectly affect the server module, which feels kind of yucky to me.

[tabazevedo]

Hmm.. it made sense in my head. Hardcoded stuff takes priority over dynamic env stuff which takes priority over defaults.

[charypar]

@chrisvfritz yeah, that's because the sever binary just doesn't take any and pass them in to the server module. That's kind of what I meant... we should pass options in in all cases, even when they come from environment and otherwise use defaults. There shouldn't be a middle ground, i.e. using an environment variable is a caller's choice, not a server implementation behaviour. Hope that makes sense, it's a really tiny nuance :)

[chrisvfritz]

I agree with @charypar. And I actually like the override priority you're using @tabazevedo, it's just a question of Where Should Stuff Go. 😃