Skip to content

Instantly share code, notes, and snippets.

@cklanac
Created December 18, 2017 13:21
Show Gist options
  • Save cklanac/d054ba9f1331e62a5fb76e8e402f0ca6 to your computer and use it in GitHub Desktop.
Save cklanac/d054ba9f1331e62a5fb76e8e402f0ca6 to your computer and use it in GitHub Desktop.
NPM script commands at various phases of the course

npm-run-setups

The primary question is whether npm start and npm test should be setup for developer convenience and other commands like npm run heroku and npm run travis should be setup for services. Or should npm start and npm test should be kept for production and test environments respectively and customer scripts like npm run dev:start and npm run dev:test should be created for development.

The secondary question is... what should they be named?

Below are examples of the scripts at the various phases of the course

Basic - Node/Express

This version would be used at the beginning of the course, before testing has been introduced

"scripts": {
  "start": "node server.js"
},

Introduce this setup during the during express but before mocha

"scripts": {
  "start": "node server.js",
  "dev:start": "nodemon server.js"
}

Intermediate: Mocha Testing

Introduce this setup for mocha testing

  • Keep npm start and npm test reserved for Heroku and Travis. Create npm run dev:start and npm run dev:test scripts for development.
"scripts": {
  "start": "node server.js",
  "test": "cross-env NODE_ENV=test mocha",
  "dev:start": "nodemon server.js",
  "dev:test": "nodemon --exec npm test"
},

The proposed solutions use process.env.NODE_ENV to determine the correct database connection and suppress logging. It is considered bad practice to set process.env.NODE_ENV in code or use globals like so we'll set it in the command using cross-env so it works cross platform. This also assumes the use of a .env file and the dotenv package for database credentials and jwt secrets.

Side Note: Never use mocha --watch. It may run faster but it causes a the app to create multiple processes and database connections. IMHO, the overhead of adding graceful shutdown scripts for --watch (which don't work for SIGINT or SIGTERM) is not worth the effort.

Advanced: Test Coverage

Propose: Introduce test coverage using nyc formerly known as istanbul as an advanced/optional lesson.

"scripts": {
    "start": "node server.js",
    "test": "cross-env NODE_ENV=test mocha",
    "dev:start": "nodemon server.js",
    "dev:test": "nodemon --exec npm test",
    "dev:cover": "nodemon --exec nyc --reporter=lcov --reporter=text-summary npm test"
  },
@cklanac
Copy link
Author

cklanac commented Dec 18, 2017

Alternatively, we keep npm start and npm test for development and use nodemon for file watching. Then create custom scripts for Travis and Heroku. This has the benefit of being easy to type (npm start opposed to npm run dev:start) but adds complexity/fragility to the setup.

"scripts": {
    "start": "nodemon server.js",
    "test": "cross-env NODE_ENV=test nodemon --exec mocha",
    "cover": "nodemon --exec nyc --reporter=lcov --reporter=text-summary npm test",
    "heroku": "node server.js",
    "travis": "nyc mocha"
  },

This setup requires a Procfile for Heroku to run the right command. It's a way good to introduce Procfiles but it is a bit more complex and fragile.

Procfile

web: npm run heroku

And it requires an additional line in the .travis.yml. Again, a good teaching opportunity, but fragile.

.travis.yml

language: node_js
node_js: node
script: npm run travis
deploy:
  provider: heroku
...

@oampo
Copy link

oampo commented Dec 18, 2017

I favour npm start and npm test to be for the developer, and npm run heroku etc. for the services. Two very minor rationales: it's consistent with CRA, so students don't need to remember something different for Node and React. And it's less to type. But I sense that this is a potential bike shed, and I really don't care much, so I'll follow consensus of opinion on this one.

@Rosuav
Copy link

Rosuav commented Dec 18, 2017

If you're using nodemon, the idea is to set it up and leave it there, so it doesn't make a lot of difference if there's a bit more to type (you're basically only doing it once per session). Reserving npm start for the most normal way to invoke your application means that anyone who downloads it doesn't have to try to figure out what YOUR particular incantation is. I like npm run dev for the nodemon usage, but other spellings are about as good.

@tsongas
Copy link

tsongas commented Dec 18, 2017

I like the idea of bringing dotenv into the curriculum, I think it's essential. The last startup I worked at, I remember them having a security incident and it turned out their credentials were in their repo (this at a company of 200 people).

Regarding the start script, I owned a hosting company for ten years so I favor anything we can do to make deployment more standard and redundant. To me that means "start": "node server.js" which works by default, and I like the idea of introducing a Procfile but I'd say let's do it for the sake of introducing forever as an optional way of improving availability, and leave "start": "node server.js" in place as a fallback.

I also like npm run dev because to me "dev": "nodemon server.js" makes it super obvious how to start the server in development mode. I've only seen semi-colons used in script names when those scripts are being called by other scripts, but maybe I just haven't seen enough projects and need to get used to the idea.

Personally, I wouldn't want my tests to run every time I save a file, I don't need that kind of anxiety! Plus, I think a good linting setup alleviates a lot of the need for that. So, I can do without a dev:test script and would just want a script like "cover": "nyc --reporter=lcov --reporter=text-summary npm test" for test coverage. However I get that's a personal preference and it's cool that nodemon can be used for more than just restarting Node.

@cklanac
Copy link
Author

cklanac commented Dec 19, 2017

@tsongas, I believe momentjs is for data/time manipulation, so I assume you mean forever... but I get your point.

I'd be happy to jettison the auto run of tests on change. I only added it in because students were using the --watch flag which I consider broken - it causes way too many issues.

I'll also defer to consensus about npm run dev, npm run cover.

@tsongas
Copy link

tsongas commented Jan 3, 2018

@cklanac oops yes I meant forever corrected that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment