jhipster / generator-jhipster-nodejs

A NodeJS blueprint that creates the backend using NestJS
https://www.npmjs.com/package/generator-jhipster-nodejs
Apache License 2.0
257 stars 80 forks source link

Rename NODE_ENV in BACKEND_ENV with better usage documentation #206

Closed dancancro closed 3 years ago

dancancro commented 3 years ago

Describe the bug When attempting to start the server of a brand new application, server fails to start because NODE_ENV is not set to "dev", "test" or "prod"

To Reproduce Generate a new application. Set NODE_ENV manually on the command line by typing NODE_ENV=elephant Try to start the backend server with npm run start Get this error

[nodemon] starting `ts-node src/main.ts`
[Nest] 18366   - 03/16/2021, 9:51:53 PM   [Config] Actual process.env.NODE_ENV value: elephant
[Nest] 18366   - 03/16/2021, 9:51:53 PM   [Config] Standard allowed values are: dev, test or prod
[Nest] 18366   - 03/16/2021, 9:51:53 PM   [Config] does not exist under your config folder an application-{process.env.NODE_ENV}.yml file with your process.env.NODE_ENV value +1ms

Set NODE_ENV to dev by typing NODE_ENV=dev on the command line. Run npm run start and the application starts

Try running it in production mode by npm run build npm run start:prod NODE_ENV is still dev but should be prod

[Nest] 20230   - 03/16/2021, 10:50:03 PM   [Config] Actual process.env.NODE_ENV value: dev
[Nest] 20230   - 03/16/2021, 10:50:03 PM   [Config] Standard allowed values are: dev, test or prod
[Nest] 20230   - 03/16/2021, 10:50:03 PM   [NestFactory] Starting Nest application... +433ms

Expected behavior The server starts without errors and in the correct mode.

Desktop (please complete the following information):

NHipster configuration

JHipster Version(s)
rfrchat@0.0.1-SNAPSHOT /Users/Dan/work/c/jhip-rfr
├── generator-jhipster@6.8.0
└─┬ generator-jhipster-nodejs@1.4.0
  └── generator-jhipster@6.8.0  deduped
JHipster configuration, a .yo-rc.json file generated in the root folder
.yo-rc.json file
{
  "generator-jhipster": {
    "jhipsterVersion": "6.8.0",
    "applicationType": "monolith",
    "baseName": "rfrchat",
    "packageName": "com.jhipster.node",
    "packageFolder": "com/jhipster/node",
    "serverPort": "8081",
    "authenticationType": "jwt",
    "cacheProvider": "no",
    "enableHibernateCache": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "sqlite",
    "prodDatabaseType": "postgresql",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSwaggerCodegen": false,
    "embeddableLaunchScript": false,
    "useSass": true,
    "clientPackageManager": "npm",
    "clientFramework": "react",
    "clientTheme": "none",
    "clientThemeVariant": "",
    "creationTimestamp": 1615934026229,
    "testFrameworks": ["protractor"],
    "jhiPrefix": "jhi",
    "entitySuffix": "",
    "dtoSuffix": "DTO",
    "otherModules": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.4.0"
      }
    ],
    "enableTranslation": false,
    "blueprints": [
      {
        "name": "generator-jhipster-nodejs",
        "version": "1.4.0"
      }
    ]
  }
}

JDL for the Entity configuration(s) entityName.json files generated in the .jhipster directory
JDL entity definitions

Environment and Tools

java version "1.8.0_131" Java(TM) SE Runtime Environment (build 1.8.0_131-b11) Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

git version 2.10.1

node: v10.20.1

npm: 6.14.4

yeoman: 1.7.0

yarn: 1.17.3

Additional context Add any other context about the problem here.

ghost commented 3 years ago

Thanks for the interest. However is correct to have only three values for NODE_ENV (dev, prod and test). The error is implemented ad hoc for that. Besides, if you run npm run start:prod the NODE_ENV is no set, the command is for other things. Please read project README.md. Thanks

dancancro commented 3 years ago

@amanganiello90 Thank you. I had not seen the file jhipster-sample-app-nodejs/blob/v1.4.0/server/README.md.

I would like to provide some feedback as someone who just saw this project for the first time yesterday. I am accustomed to finding all instructions in the top level README.md. So it is foreign to me for usage instructions to be somewhere else, especially inside of a sample app. Adding a comment to the top level README.md that instructions exist somewhere else would be very helpful.

I see the instruction set NODE_ENV=prod&& npm run build && npm run start:prod and think that this could be made into a single npm script. I don't think you would ever run the second or third parts of this with NODE_ENV=dev, would you? Certainly not npm run start:prod. The same seems to be true for set NODE_ENV=prod&& node dist/main.js

It seems a bit confusing to me that NODE_ENV can be set by default. Its value should necessarily be prod, dev, or test, according to which npm script you are running. For the sake of simplicity and uniformity, I would remove the .env file and explicitly set NODE_ENV in each applicable npm script of package.json.

ghost commented 3 years ago

Hi @dancancro, I will answer every your doubts: 1- There is a reference for the server/README.md in the top level README project -> https://github.com/jhipster/jhipster-sample-app-nodejs#building 2- You are confusing about the sense of the npm run start:prod. It performs a run of an already javascript server build, but the default environment (NODE_ENV) is dev. So it runs for example with a sqlite database, while if you set the NODE_ENV=prod you will run your app with a prod database -> https://github.com/jhipster/jhipster-sample-app-nodejs/tree/main/server#define-your-prod-database 3- For the point 2, you can run npm run start or npm run start:prod with or without NODE_ENV=prod, as they are indipendent (NODE_ENV is used to decide what database you want at runtime, while the other scripts specify the build mode (transpile the ts source files and run, with the npm run start script, or run the javascript js files already transpiled previously with the npm run start:prod command) 4- The .env file is used to have in the entire app the default valuefor NODE_ENV that is dev. To avoid that the user could set values not eligible, there is a control that rises an error (the NODE_ENV must be dev, prod, or test). 5- At the startup, there is a log for NODE_ENV value. Dev or prod value is used for the database setup, while test value for context unit test running. If you might set an other value (for example foo) the app will crash for database and other reasons. So the check that raises the error is to avoid not allowed NODE_ENV values.

I hope that now it's all clear. Thanks for the issue and questions.

dancancro commented 3 years ago

I am referring to the top level readme of generator-jhipster-nodejs.

This one: https://github.com/jhipster/generator-jhipster-nodejs not this one: https://github.com/jhipster/jhipster-sample-app-nodejs

Thank you for clarifying the use of NODE_ENV for setting which database is used.

This seems to muddy the meaning of NODE_ENV. "ENV" is short for "environment". An environment is a totality of conditions. When I say I am running in one environment or another, I take that to mean that everything is one way or another.

I think that fine grain control of which database is used, is a different matter than "environment" and therefore it should be controlled by something other than NODE_ENV, perhaps NODE_DATABASE, or just DATABASE

  1. I am unclear what "entire app" means in "have in the entire app the default value for NODE_ENV that is dev".

This is a really beautiful project and it is necessarily complex. NODE_ENV therefore has many different effects on things. As a brand new viewer of this project, the complete consequences of this variable were not clear to me. I think that this could be improved with a combination of documentation and possibly changing how the value is set or adding additional variable(s) for controlling the various things that it controls now. It is also a bit confusing that the front end NODE_ENV values are "development" and "production" but the back end values are "dev" and "prod". NODE_ENV just seems to be asked to do way too much for one variable.

Finally, I wanted to add that in my own project we struggled deciding how to do environment variables. We have some secret keys that are unique to each developer. So they can't be checked in. We use dotenv to read those from untracked .env files, but we get NODE_ENV from package.json scripts. I'm still not sure we did it the right way.

ghost commented 3 years ago

I try to give you clear information:

  1. There isn't sense to explain on the top level of generator-jhipster-nodejs readme what are the build information for the app generated (it is the README generator, where you read information about run of the generator). Therefore you generate the app, and then on the README generated you read all the information about the app generated (build, dts, run and so on)
  2. The NODE_ENV is for environment as you explain. Now It used for database instance and build webpack mode, tomorrow it will give other customization
  3. have in the entire app the default value for NODE_ENV that is dev means that for the backend you have a default NODE_ENV value as dev for many configuration (for example for database istance)
  4. The NODE_ENV value (dev, prod or test) it's not different from frontend: for example in angular you can specify --prod or --production option that is an alias (also in webpack)
  5. I don't understand what is the problem to have a default value in your dot env file. You can add your secrets in that file, and if your backend run with a prod database (or you want to use webpack in prod), set the value of NODE_ENV in that file with prod

However, I will do the following actions to improve the generator:

Thanks for the feedbacks.

ghost commented 3 years ago

Hi @dancancro, I have improved the documentation and renamed NODE_ENV in BACKEND_ENV. It has been published the new 1.5.0 generator version with that and other features that you can read on Release CHANGELOG. For the new app documentation you can refers to the generated example server README.

Thanks for the interest!!