Use process.env for PORT

[erikkrieg]

I'm submitting a...

• Regression
• Bug report
• Feature request
• Documentation issue or request
• Support request

Current behavior

Starting a new project includes an anti-pattern concerning the handling of configuration. Specifically, src/main.ts has the app listen on a hardcoded port.

E.g.

async function bootstrap() {
  const app = await NestFactory.create(AppModule);
  await app.listen(3000);
}
bootstrap();

On its own this is a seemingly small thing, but it elevates the risk of projects neglecting one of the tenants of The Twelve-Factor App:

Store config in the environment

https://12factor.net/config

A consumer of the framework would need to be aware of this anti-pattern and discover the documentation recommending how to properly handle configuration with Nest.

Expected behavior

Instead of the app listening to a hardcoded port, use a ConfigService class as suggested in the docs to pull the port from the environment.

Documentation should also link users to the page on configuration techniques.

Minimal reproduction of the problem with instructions

Start a new project with Nest's CLI (nest new project) and see that src/main.ts has hardcoded port.

What is the motivation / use case for changing the behavior?

Two motivations for this feature request:

1. Mitigate risk of applications being built on top of an anti-pattern.
2. Educate developers who are newer to backend development by putting best practices in the critical path of using Nest.

Environment

@nestjs/cli: 5.8.0
@nestjs/common: 5.4.0
@nestjs/core: 5.4.0

[erikkrieg]

I also just wanted to thank the contributors of this project for working towards addressing the lack of strong convention in the Node server-side ecosystem ❤️

[kibertoad]

@erikkrieg Please review an example in nestjs/nest#1573
After this is polished and merged, same changes could be applied to Nest CLI as well.

[kamilmysliwiec]

Thank you @erikkrieg.

Just to explain. Using ConfigService outside of the application context is possible, but is not (always) considered as a best practice. Basically, ConfigService should be used within your application so you can easily mock/override it in your tests. Nonetheless, there is nothing wrong with using process.env directly in your main.ts which you will very likely don't test at all.

[erikkrieg]

I am unsure why the issue has been closed. Might be a misunderstanding on what I was trying to communicate, or perhaps something I am missing about the CLI.

I don't really know this framework well enough to have an opinion on how this project handles configuration so I referred to the ConfigService class because that is what is used in the docs to showcase this best practice.

To clarify the issue I was pointing to, my concern is that the CLI creates a new project that lacks a good convention for handling configuration. In this case, the port that the server listens on. This implicitly promotes an anti-pattern and misses an opportunity to share the opinions that project already seems to have on how to handle configuration.

The Node ecosystem is sorely missing a strong, opinionated framework like what Rails is to Ruby. I hope that this project is aspiring to fill that void. I think having the CLI get devs started without having to think about setting conventions like this helps get there.

[kamilmysliwiec]

@erikkrieg

In this case, the port that the server listens on

Well, that might be a good thing actually. I thought you wanted to suggest the usage of the ConfigService explicitly. I think that we could change the schematic, so it will generate this:

app.listen(process.env.PORT || 3000)

(instead of just hardcoded port)

[erikkrieg]

Yeah, that is a step in the direction I was trying to communicate. Ofc it is up to the maintainers to determine how sophisticated the handling of config in the "basic" project generated from the CLI should be.

[Nigelli]

Hey firstly @kamilmysliwiec, great job on this project; I'm enjoying working with it :).

Using something like app.listen(process.env.PORT || 3000) assumes that the env variable is configured and available for this app. Considering this schematic is likely to be used for creating multiple projects we will likely still need to end up changing this.

I've happened to solve this for our custom schematic that extends this in order to add some business specifics.

By utilizing environments files(As Angular does) and schema properties in combination, we are able to generate and run multiple nest apps straight from the cli with no additional config. I think something along these lines could provide an elegant solution.

Example:

We set the environment.prod file to expose a server_port property to a environment variable.
export const environment = { production: true, server_port: process.env'<%=name%>-port'] };

And in the dev environment file we used a port chosen at the generate stage.
export const environment = { production: false, server_port: '<%=port%>' };

The schema.json sets the default and prompts for a choice
"port": { "description": "The dev port this server should run on.", "type": "string", "default": "3333", "x-prompt": "What port would you like to use for the app?" },

In our main.ts we just import import { environment } from './environments/environment'; and use it like app.listen(environment.server_port, () => {});

[felixhayashi]

Actually, there is a popular npm package (https://github.com/lorenwest/node-config) that allows you to define config files for different "environments" and at runtime it will pick the config matching NODE_ENV. I think this sufficiently abstracts the configuration process and makes it fairly easy to switch environments (see the README examples). As @kamilmysliwiec noted, it is fine that this aspect of the configuration mechanism is not provided as a service since the bootstrap() is out of the DI-scope anyways. Of course, nonetheless, nothing prevents you from wrapping the config mechanism in a service to provide it to other parts of your app later.

[siddrc]

@felixhayashi yes this works ....thank you so much, especially the fact that node-config has a feature of
Custom Env variables
It took me a while to realise that I need to keep the config folder outside the src folder and then it worked beautifully. Thank you

[j1i-ian]

Many people use .env but I have been not saw who think about why we should use .env at first time and why there are people that use as .env.bak.
Generally sensitive data should be not exposed in some file or and should be not tracked.

Of source, I can understand always setting vars is to be tedious too (or maybe we should patch encrypted vars dynamically with big working).
But tracking or maintaining env vars on source, it means injector is not only one like pm2 ecosystem json or something in future. then it also means config service cannot be fixed.

Moreover, Nowadays on javascript, pm2 is used on dev / staging env normally.

Maybe you don't need to use .env except on local env.
Finally I think, it is ok this issue is closed.

[ryakoviv]

export class AppModule {
   static port: string
  constructor(configService: ConfigService) {
    AppModule.port = configService.get('HTTP_PORT')
  }
}

async function bootstrap() {
  const app = await NestFactory.create(AppModule);
  await app.listen(AppModule.port || 3000);
}
bootstrap();

[ahmed-shadab]

I have gone through official nestjs documentation. Here is what should be done
in main.ts-

import { ConfigService } from '@nestjs/config';

const port = configService.get('PORT');
await app.listen(port|3000);

in .env
PORT=3000

In app.module.ts , import config module if not done ;-)
@module({
imports: [
ConfigModule.forRoot({ isGlobal: true }),
]
})