Merge pull request #149 from ampproject/logging

Improve Debugging & Configuation

README.md

@@ -72,7 +72,7 @@ This codebase adheres to the [Googe Javascript Styleguide](https://google.github
 npm run lint
 
 # run ESLint with `--fix` option to automatically fix errors (if possible)
-npm run lint-fix
+npm run lint:fix
 ```
 
 ## Contributing

package.json

@@ -7,7 +7,7 @@
     "test:node": "jasmine --config=jasmine.json",
     "test:browser": "karma start --single-run --browsers ChromeHeadless karma.conf.js",
     "lint": "eslint \"**/*.js\" --ignore-path .gitignore",
-    "lint-fix": "eslint \"**/*.js\" --fix --ignore-path .gitignore"
+    "lint:fix": "eslint \"**/*.js\" --fix --ignore-path .gitignore"
   },
   "devDependencies": {
     "@ampproject/rollup-plugin-closure-compiler": "0.8.3",

packages/cli/index.js

@@ -17,15 +17,14 @@
 'use strict';
 
 const Cli = require('./lib/cli');
-const Logger = require('./lib/util/Logger');
+const {log} = require('amp-toolbox-core');
 
 module.exports = () => {
-  const logger = new Logger();
-  const cli = new Cli(logger);
+  const cli = new Cli(log);
   const args = process.argv.slice(2);
   cli.run(args)
       .catch((err) => {
-        logger.error(err.message);
+        log.error(err.message);
         process.exit(1);
       });
 };

packages/cli/lib/cli.js

@@ -17,11 +17,11 @@
 'use strict';
 
 const minimist = require('minimist');
-const Logger = require('./util/Logger');
+const {log} = require('amp-toolbox-core');
 
 class Cli {
-  constructor(logger = new Logger()) {
-    this.logger = logger;
+  constructor(logger=log) {
+    this.logger_ = logger;
   }
 
   run(args) {
@@ -30,11 +30,11 @@ class Cli {
 
     switch (command) {
       case 'help':
-        return require('./cmds/help')(args, this.logger);
+        return require('./cmds/help')(args, this.logger_);
       case 'version':
-        return require('./cmds/version')(args, this.logger);
+        return require('./cmds/version')(args, this.logger_);
       case 'update-cache':
-        return require('./cmds/updateCache')(args, this.logger);
+        return require('./cmds/updateCache')(args, this.logger_);
       default:
         return Promise.reject(new Error(`"${command}" is not a valid command!`));
     }

packages/cli/lib/cmds/updateCache.js

@@ -36,14 +36,14 @@ function updateCaches_(privateKey, url, logger) {
 }
 
 function updateCache_(cacheUpdateUrlInfo, logger) {
-  const tag = cacheUpdateUrlInfo.cacheName;
-  logger.info(`Invalidating ${cacheUpdateUrlInfo.cacheName}`, tag);
-  logger.info(`Using Invalidation URL: ${cacheUpdateUrlInfo.updateCacheUrl}`, tag);
+  logger = logger.tag(cacheUpdateUrlInfo.cacheName);
+  logger.info(`Invalidating ${cacheUpdateUrlInfo.cacheName}`);
+  logger.info(`Using Invalidation URL: ${cacheUpdateUrlInfo.updateCacheUrl}`);
 
   fetch(cacheUpdateUrlInfo.updateCacheUrl)
       .then((response) => {
         if (response.status === 200) {
-          logger.success(`${cacheUpdateUrlInfo.cacheName} Updated`, tag);
+          logger.success(`${cacheUpdateUrlInfo.cacheName} Updated`);
           return;
         }
         return response.text()
@@ -52,18 +52,18 @@ function updateCache_(cacheUpdateUrlInfo, logger) {
               if (match) {
                 logger.error(
                     `Error Invalidating Cache URL. Received response code "${response.status}" ` +
-                    `with message: "${match[1]}"`, tag
+                    `with message: "${match[1]}"`
                 );
               } else {
                 logger.error(
                     `Error Invalidating Cache URL. Received response code "${response.status}"` +
-                    'with an unknown error', tag
+                    'with an unknown error'
                 );
               }
             });
       })
       .catch((e) => {
-        logger.warn(`Error connecting to the AMP Cache with message: "${e.message}"`, tag);
+        logger.warn(`Error connecting to the AMP Cache with message: "${e.message}"`);
       });
 }
 

packages/core/index.js

@@ -18,5 +18,6 @@
 module.exports = {
   MaxAge: require('./lib/MaxAge.js'),
   OneBehindFetch: require('./lib/OneBehindFetch.js'),
+  log: require('./lib/Log.js'),
 };
 

packages/core/lib/Log.js

@@ -0,0 +1,70 @@
+class Log {
+  constructor(tag='', verbose=false, output=console) {
+    this.tag_ = tag;
+    this.verbose_ = verbose;
+    this.prefix_ = this.inverse_(tag);
+    this.output_ = output;
+  }
+
+  debug(message, ...args) {
+    if (!this.verbose_) {
+      return;
+    }
+    this.log_(this.output_.log, this.dim_(message), args);
+  }
+
+  info(message, ...args) {
+    this.log_(this.output_.log, message, ...args);
+  }
+
+  warn(message, ...args) {
+    this._log(this.output_.warn, this.yellow_('WARNING ' + message), args);
+  }
+
+  error(message, ...args) {
+    this.log_(this.output_.error, '\n');
+    this.log_(this.output_.error, this.red_('ERROR ' + message), args);
+    this.log_(this.output_.error, '\n');
+  }
+
+  verbose(isVerbose=true) {
+    this.verbose_ = Boolean.valueOf(isVerbose);
+  }
+
+  tag(newTag) {
+    if (this.tag_) {
+      newTag = this.tag_ + ' ' + newTag;
+    }
+    return new Log(newTag, this.verbose_, this.output_);
+  }
+
+  log_(fn, message, args) {
+    if (this.prefix_) {
+      message = this.prefix_ + ' ' + message;
+    }
+    if (args) {
+      fn(...[message].concat(args));
+    } else {
+      fn(message);
+    }
+  }
+
+  inverse_(string) {
+    return `\x1b[7m${string}\x1b[0m`;
+  }
+
+  dim_(string) {
+    return `\x1b[36m${string}\x1b[0m`;
+  }
+
+  yellow_(string) {
+    return `\x1b[33m${string}\x1b[0m`;
+  }
+
+  red_(string) {
+    return `\x1b[31m${string}\x1b[0m`;
+  }
+}
+
+module.exports = new Log();
+

packages/optimizer/README.md

@@ -220,6 +220,26 @@ A few notable changes are:
 
 ## Best Practices
 
+### Debugging
+
+Enable `verbose` mode to find out why the AMP boilerplate is not being removed.
+
+```
+// globally
+ampOptimizer.setConfig({
+  verbose: true
+});
+ampOptimizer.transformHtml(originalHtml, {
+  ampUrl: 'canonical.amp.html'
+})
+
+// per transformation
+ampOptimizer.transformHtml(originalHtml, {
+  ampUrl: 'canonical.amp.html',
+  verbose: true
+})
+```
+
 ### Transform AMP pages at build time if possible
 
 Applying the transformations to an AMP file consumes additional server

packages/optimizer/index.js

@@ -17,26 +17,4 @@
 
 const DomTransformer = require('./lib/DomTransformer.js');
 
-const defaultConfig = {
-  transformers: [
-    // Adds a link to the valid AMP version
-    'AddAmpLink',
-    // Applies server-side-rendering optimizations
-    'ServerSideRendering',
-    // Removes ⚡ or 'amp' from the html tag
-    'RemoveAmpAttribute',
-    // Removes the boilerplate
-    // needs to run after ServerSideRendering
-    'AmpBoilerplateTransformer',
-    // Optimizes script import order
-    // needs to run after ServerSideRendering
-    'ReorderHeadTransformer',
-    // needs to run after ReorderHeadTransformer
-    'RewriteAmpUrls',
-    'GoogleFontsPreconnect',
-    'PruneDuplicateResourceHints',
-    'AddBlurryImagePlaceholders',
-  ],
-};
-
-module.exports = new DomTransformer(defaultConfig);
+module.exports = new DomTransformer();