General: process.env

[kettanaito]

As I have noticed, we can reuse process.env Object to write and read environment type from it. This made me thinking, what if reuse environment from it directly, without having to include config or other files each time we use environment check.

// environment.js
const argv = require('yargs').argv;
const DEVELOPMENT = 'development';
const PRODUCTION = 'production';
const isDevelopment = argv.dev || false;
environmentType = isDevelopment ? DEVELOPMENT : PRODUCTION;

/* Update Node environment */
process.env.NODE_ENV = environmentType;
process.env.isDevelopment = isDevelopment;
process.env.isProduction = !isDevelopment;

/* Exported Object is currently used by eslint-actum-config */
/* but I would also suggest to refactor that fact to get rid of this export  */
module.exports = {
    type: environmentType,
    isDevelopment,
    isProduction: !isDevelopment
};

// some-file.js
const environmentType = process.env.NODE_ENV;
const DEVELOPMENT = process.env.isDevelopment;

This way each time you need to know of the current environment, it is stored in process.env, a global object.

What do you think?

[janpanschab]

1) Just setup process.env.NODE_ENV correctly based on argv.dev and then inside modules/tasks use process.env.NODE_ENV.

2) Use it as it is now.

I’m OK with both approaches. cc @vbulant ?

[vbulant]

I'm for 1. if it decreases amount of our code. The less code, the better.

[kettanaito]

@janpanschab this is what I am suggesting in this issue (1st option).

[janpanschab]

@kettanaito Then OK! 👍

[kettanaito]

@janpanschab if I understand you correctly, you want to setup NODE_ENV value and utilize it. So the further usage looks like:

// some-setting-file.js
const environment = process.env.NODE_ENV;
...
if (environment === 'DEVELOPMENT') {
    console.log('I am on development');
}

Which is technically fine, but will lead to a lot of repetitive code since each comparison should be whether done manually or stored under certain scope. This is why I would suggest to make this check at the same time when updating the process object:

// environment.js
process.env.NODE_ENV = isDevelopment ? 'DEVELOPMENT' : 'PRODUCTION';
process.env.isDevelopment = isDevelopment;
process.env.isProduction = !isDevelopment;

This way we would put the logic into globalish scope through global process object. process.env.NODE_ENVis generally needed for third-party libraries to compile correspondingly to the current environment. At the same timeprocess.env.isDevelopment/isProduction` are going to be utilized by us:

// some-setting-file.js
const is Development = process.env.isDevelopment;
...
if (isDevelopment) {
    console.log('I am on development');
}

[vbulant]

What’s the motivation for the separation? I don’t see any advantage of having process.env.isDevelopment instead of using process.env.NODE_ENV directly…

[kettanaito]

@vbulant to prevent from constant checking of process.env.NODE_ENV. Since this is a string, you would need to check each time whether it is equal to "development" or "production". Or create a logic somewhere on top of the setting file, for example:

// some-setting-file
const isDevelopment = (process.env.NODE_ENV === 'development');

The main motivation is the availability scope. If we use NODE_ENV directly, we never know under which environment we are unless we check it. What I suggest is to check it once, and then utilize (this was the original idea of creating environment.js file to handle this logic). This is again, just to remove the repetitive lines of code from each setting file.

[vbulant]

Yep, I agree with having something like const isDevelopment = (process.env.NODE_ENV === 'development');

I just didn’t see the difference between checking process.env.NODE_ENV or process.env.isDevelopment…

OK, now we’re on the right track :)

[kettanaito]

After a continuous investigation, there is an issue with process.env.CUSTOM_VARIABLE when using on server side (NodeJS). The specification states that all custom properties under process.env should always be string. In case they are not, they are converted to the string.

This raises the issue when trying to use process.env.isDevelopment on server side. For client this reference will return boolean:

process.env.isDevelopment = true;

For server, however, this expression will equal to String, with both "true" and "false" converted to true (any non-empty string is considered true).

We should take this into account when implementing the suggested changes. If we do not plan by any chance improving our devstack by NodeJS (standalone API, helper server, etc.), then there is no obstacle.

What do you say?

[kettanaito]

Actually, regarding environment handling, I have one idea and will introduce it under Webpack 2 (2.0) branch. I will try to combine simplicity of this suggestion, but take into account that anything under process.env is always a string, and will always resolve to true when used not carefully.

I think we can close this issue.