onedesign / generator-one-base

A foundation for One Design Company projects on the web.
Other
1 stars 1 forks source link

Add `NODE_ENV` to package.json tasks #66

Closed brianjhanson closed 7 years ago

brianjhanson commented 7 years ago

Changing our package.json to:

  "scripts": {
    "start": "NODE_ENV=development gulp",
    "build": "NODE_ENV=production gulp build"
  }

Will allow us to update how our tasks behave on development vs. production. Very useful if react is in a project.

mkornatz commented 7 years ago

I don't know how I feel about setting the NODE_ENV via these scripts. I feel like we could run into trouble with them down the line.

If anyone/everyone else is comfortable with it, though, instead of using "start" which is a common way to start apps in production mode, could we use "develop" or "watch" as the script name?

brianjhanson commented 7 years ago

Good point, especially if we're actually building something with node. My main thought about doing it there is because then it's a little easier to test Production stuff locally (running yarn build would pretty accurately simulate production). Moving this into the .env file might be better though.

I'd be find with renaming yarn start to yarn develop. I'm not sure how many of us actually use that command anyways (our projects aren't quite consistent enough to have it deeply engrained yet)

mkornatz commented 7 years ago

I think we'd have to run NODE_ENV=production yarn build rather than relying on the .env file, because that way it gets set before yarn even initializes. I'm not completely sure if that matters in yarn land, but in the DevOps world, it's quite common to prefix a command with NODE_ENV=xxx.

cmalven commented 7 years ago

I don't have strong feelings about most of this, but I do feel like continuing to use yarn start or npm start to kick off app development is valuable given how well established that pattern is in the node community. I think it's also one of the only scripts you can run without using run (e.g. yarn start vs yarn run start) so it's a little bit shorter.

brianjhanson commented 7 years ago

Ah @mkornatz, are you saying go with:

  "scripts": {
    "start": "gulp",
    "build": "NODE_ENV=production gulp build"
  }

rather than my original proposal?

mkornatz commented 7 years ago

For non-node apps, I agree @cmalven. For node apps, though, we're probably not going to be using gulp to run the app in production. At that point, it would become something like node main.js or something.

mkornatz commented 7 years ago

@brianjhanson, yeah, I'm recommending removing environment vars from the scripts altogether. Hard-coding NODE_ENV=production means you'd never be able to override it for another environment. When we need to build for prod (if it's different from dev), then we'd add an ENV var for NODE_ENV or set it explicitly when running the command e.g. NODE_ENV=production yarn build.

brianjhanson commented 7 years ago

ahhh, I get ya.

brianjhanson commented 7 years ago

So it seems like rather than change this here, we should make sure we set up our buddy task to run NODE_ENV=production yarn build rather than yarn build (or we could set it as a buddy environment variable). I still think it's useful to tweak how we run our tasks based on the environment.

cmalven commented 7 years ago

What is the resolution for this issue? Is there a place in the wiki we can include this detail about setting the NODE_ENV?

cmalven commented 7 years ago

Closing this. If there's still any outstanding tasks associated with this please add a comment describing them.