Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added NODE_ENV to configure debug mode #80

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wardpeet
Copy link

@wardpeet wardpeet commented May 2, 2016

Make debug flag editable by setting NODE_ENV to production.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.4% when pulling b12898c on wardpeet:debugmode into bbdc191 on wilsonpage:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.4% when pulling 640d02e on wardpeet:debugmode into bbdc191 on wilsonpage:master.

@wardpeet
Copy link
Author

This would be handy when you use it with webpack, you can trigger debug mode

@pke
Copy link

pke commented Nov 29, 2017

Good patch to make the production bundle as small as possible. I wonder why the compiled libs are checked into the repo though. Shouldn't they just be deployed to npm?

@wardpeet
Copy link
Author

no idea but repo has this so I thought i would ust do the same

@pke
Copy link

pke commented Nov 29, 2017

@wilsonpage why are the compiled versions checked in?
@wardpeet maybe you can rebase and @wilsonpage can merge this then?

@wilsonpage
Copy link
Owner

@pke yes the compiled /fastdom-strict.js is checked in, just so consumers have the choice to load via <script> if need be.

@@ -18,7 +18,7 @@
*
* @return {Function}
*/
var debug = 0 ? console.log.bind(console, '[fastdom]') : function() {};
var debug = process.env.NODE_ENV !== 'production' ? console.log.bind(console, '[fastdom]') : function() {};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to throw if users aren't using webpack/browserify.

@@ -5,7 +5,7 @@
*
* @return {Function}
*/
var debug = 0 ? console.log.bind(console, '[fastdom-sandbox]') : function() {};
var debug = process.env.NODE_ENV !== 'production' ? console.log.bind(console, '[fastdom-sandbox]') : function() {};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to throw if users aren't using webpack/browserify

@@ -64,7 +64,7 @@ return /******/ (function(modules) { // webpackBootstrap
*
* @return {Function}
*/
var debug = 0 ? console.log.bind(console, '[fastdom-strict]') : function() {};
var debug = false ? console.log.bind(console, '[fastdom-strict]') : function() {};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this different to the change in /fastdom.js?

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.

4 participants