Skip to content

Use NODE_ENV (or fall back to 'production' if not set) when running build#279

Open
chrisdwheatley wants to merge 1 commit into
choojs:masterfrom
chrisdwheatley:node-env-prod
Open

Use NODE_ENV (or fall back to 'production' if not set) when running build#279
chrisdwheatley wants to merge 1 commit into
choojs:masterfrom
chrisdwheatley:node-env-prod

Conversation

@chrisdwheatley

@chrisdwheatley chrisdwheatley commented Oct 11, 2017

Copy link
Copy Markdown

As suggested by @yoshuawuyts in choojs/create-choo-electron#1 this PR sets NODE_ENV to 'production' by default when running bankai build.

If desired a user can set their own NODE_ENV which will override the default.

I tested this in create-choo-app by running npm run dev and being able to see choo-log output in the console. I then ran npm run buildand served the built files statically and saw no more logs from choo-log in the console (due to the NODE_ENV === 'production' check in create-choo-app).

I also ran NODE_ENV="chugga-chugga-choo-choo" npm run build which meant the 'production' default was overridden and this resulted in choo-log being used as expected.

Tests seem to be failing in CI so I don't think I've regressed anything but happy to take a look at that if I have (or take a look separately).

@Flet

Flet commented Oct 17, 2017

Copy link
Copy Markdown
Collaborator

I think this is great! Does it make sense to add a small note to the README about this? It could lead to surprising behavior for folks who rely on NODE_ENV in their apps, so leaving a breadcrumb would be prudent :)

@chrisdwheatley

chrisdwheatley commented Oct 20, 2017

Copy link
Copy Markdown
Author

Thanks for the feedback. I'm personally not sure the readme would be the best place for this information, perhaps a better place would be within the cli output when running bankai build, similar to;

log.info('Compiling & compressing files\n')

But along the lines of Running in ${process.env.NODE_ENV} mode\n, or something similar. What do you think?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants