Update Tailor using ES6 features(fixes #102) (#109)

* #99 Add codecov for integrating code coverage reports * #102 Add destructuring, arrow functions to first few files * #102 object literal shorthand and some other refactorings * #102 Add destructuring, () => {} to request-handler.js * #102 Add destructuring, () => {} to serializer.js * #102 Remove non-applicable TODO comment * #102 Add parameter destructuring to make filter-headers.js more concise * #102 Replace incorrect merge conflict resolution in index.js * #102 Revert parameter destructuring due to failure in node-4.x * #102 Revert lodash FP changes in filter-headers.js * #102 Update Tailor to use node 7.x * #102 Fix code comments and breaking test on node 7.x * #102 Add curried function instead of lambda
zalando · Jan 16, 2017 · 07f07dc · 07f07dc
1 parent f91bcff
commit 07f07dc
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 128 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,8 +1,8 @@
 sudo: false
 language: node_js
 node_js:
-- '4.4.4'
-- '6.0.0'
+- '6.1.0'
+- '7.1.0'
 cache:
   directories:
   - node_modules

diff --git a/index.js b/index.js
@@ -10,17 +10,21 @@ const requestFragment = require('./lib/request-fragment');
 const PIPE_DEFINITION = fs.readFileSync(path.resolve(__dirname, 'src/pipe.min.js'));
 const AMD_LOADER_URL = 'https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.22/require.min.js';
 
+
 const stripUrl = (fileUrl) => path.normalize(fileUrl.replace('file://', ''));
 const getPipeAttributes = (attributes) => {
-    const primary = (attributes.primary || attributes.primary === '') ? true : false;
-    return { primary, id: attributes.id };
+    const { primary, id } = attributes;
+    return { 
+        primary: !!(primary || primary === ''), 
+        id 
+    };
 };
 
 module.exports = class Tailor extends EventEmitter {
 
     constructor (options) {
         super();
-        const amdLoaderUrl = options.amdLoaderUrl || AMD_LOADER_URL;
+        const { amdLoaderUrl = AMD_LOADER_URL, templatesPath } = options;
         let memoizedDefinition;
         const pipeChunk = (amdLoaderUrl, pipeInstanceName) => {
             if (!memoizedDefinition) {
@@ -34,24 +38,27 @@ module.exports = class Tailor extends EventEmitter {
             }
             return new Buffer(`${memoizedDefinition}var ${pipeInstanceName}=${PIPE_DEFINITION}</script>\n`);
         };
+
         const requestOptions = Object.assign({
             fetchContext: () => Promise.resolve({}),
             fetchTemplate: fetchTemplate(
-                options.templatesPath ||
+                templatesPath ||
                 path.join(process.cwd(), 'templates')
             ),
             fragmentTag: 'fragment',
             handledTags: [],
             handleTag: () => '',
             requestFragment,
-            pipeDefinition: (pipeInstanceName) => pipeChunk(amdLoaderUrl, pipeInstanceName),
+            pipeDefinition: pipeChunk.bind(null, amdLoaderUrl),
             pipeInstanceName: () => 'Pipe',
-            pipeAttributes: (attributes) => getPipeAttributes(attributes)
+            pipeAttributes: getPipeAttributes
         }, options);
+
         requestOptions.parseTemplate = parseTemplate(
             [requestOptions.fragmentTag].concat(requestOptions.handledTags),
             ['script', requestOptions.fragmentTag]
         );
+
         this.requestHandler = requestHandler.bind(this, requestOptions);
         // To Prevent from exiting the process - https://nodejs.org/api/events.html#events_error_events
         this.on('error', () => {});

diff --git a/lib/fetch-template.js b/lib/fetch-template.js
@@ -10,13 +10,14 @@ const TEMPLATE_NOT_FOUND = 1;
 class TemplateError extends Error {
     constructor(...args) {
         super(...args);
+        this.code = TEMPLATE_ERROR;
+        this.presentable = 'template error';
+        const [{ code }] = args;
 
-        let code = TEMPLATE_ERROR;
-        if (args.length > 0 && args[0].code === 'ENOENT') {
-            code = TEMPLATE_NOT_FOUND;
+        if (code === 'ENOENT') {
+            this.code = TEMPLATE_NOT_FOUND;
             this.presentable = 'template not found';
         }
-        this.code = code;
     }
 }
 
@@ -25,28 +26,28 @@ class TemplateError extends Error {
  *
  * @param {string} path
  */
-const readFile = (path) => {
-    return new Promise((resolve, reject) => {
+const readFile = (path) => 
+    new Promise((resolve, reject) => {
         fs.readFile(path, 'utf-8', (err, data) => {
             if (err) {
                 reject(new TemplateError(err));
                 return;
-            }
+            } 
             resolve(data);
         });
     });
-};
 
 /**
  * Fetches the template from File System
  *
  * @param {string} templatesPath - The path where the templates are stored
  * @param {function=} baseTemplateFn - Function that returns the Base template name for a given page
  */
-module.exports = function fetchTemplate (templatesPath, baseTemplateFn) {
-    return (request, parseTemplate) => {
+module.exports = (templatesPath, baseTemplateFn) => 
+    (request, parseTemplate) => {
         const pathname = url.parse(request.url, true).pathname;
         const templatePath = path.join(templatesPath, pathname) + '.html';
+
         return readFile(templatePath)
             .then((baseTemplate) => {
                 if (typeof baseTemplateFn !== 'function') {
@@ -61,10 +62,9 @@ module.exports = function fetchTemplate (templatesPath, baseTemplateFn) {
                 const pageTemplate = baseTemplate;
                 const baseTemplatePath = path.join(templatesPath, templateName) + '.html';
                 return readFile(baseTemplatePath)
-                    .then(baseTemplate => parseTemplate(baseTemplate, pageTemplate));
+                    .then((baseTemplate) => parseTemplate(baseTemplate, pageTemplate));
             });
     };
-};
 
 module.exports.TEMPLATE_ERROR = TEMPLATE_ERROR;
 module.exports.TEMPLATE_NOT_FOUND = TEMPLATE_NOT_FOUND;
diff --git a/lib/filter-headers.js b/lib/filter-headers.js
@@ -1,5 +1,5 @@
 'use strict';
-
+const ACCEPT_HEADERS = ['accept-language', 'referer', 'user-agent'];
 /**
  * Filter the request headers that are passed to fragment request.
  *
@@ -8,15 +8,10 @@
  * @param {Object} headers - Request header object
  * @returns {Object} New filtered header object
  */
-module.exports = function filterHeaders (attributes, headers) {
-    const newHeaders = {};
-    if (attributes.public) {
-        return newHeaders;
-    };
-    ['accept-language', 'referer', 'user-agent'].forEach((key) => {
-        if (headers[key]) {
-            newHeaders[key] = headers[key];
-        }
-    });
-    return newHeaders;
-};
+module.exports = ({ public: publicFragment }, headers) => 
+    publicFragment 
+        ? {} 
+        : ACCEPT_HEADERS.reduce((newHeaders, key) => {
+            headers[key] && (newHeaders[key] = headers[key]);
+            return newHeaders;
+        }, {});
diff --git a/lib/parse-template.js b/lib/parse-template.js
@@ -8,10 +8,9 @@ const Transform = require('./transform');
  * @param {Array} insertBeforePipeTags - Pipe definition will be inserted before these tags
  * @returns {Promise} Promise that resolves to serialized array consisits of buffer and fragment objects
  */
-module.exports = function parseTemplate (handledTags, insertBeforePipeTags) {
-    return (baseTemplate, childTemplate) => new Promise((resolve) => {
+module.exports = (handledTags, insertBeforePipeTags) =>
+    (baseTemplate, childTemplate) => new Promise((resolve) => {
         const transform = new Transform(handledTags, insertBeforePipeTags);
         const serializedList = transform.applyTransforms(baseTemplate, childTemplate);
         resolve(serializedList);
     });
-};
diff --git a/lib/request-fragment.js b/lib/request-fragment.js
@@ -19,8 +19,8 @@ const requiredHeaders = {
  * @param {Object} request - HTTP request stream
  * @returns {Promise} Response from the fragment server
  */
-module.exports = function requestFragment (fragmentUrl, fragmentAttributes, request) {
-    return new Promise((resolve, reject) => {
+module.exports = (fragmentUrl, fragmentAttributes, request) =>
+    new Promise((resolve, reject) => {
         const parsedUrl = url.parse(fragmentUrl);
         const options = Object.assign({
             headers: Object.assign(
@@ -30,10 +30,11 @@ module.exports = function requestFragment (fragmentUrl, fragmentAttributes, requ
             keepAlive: true,
             timeout: fragmentAttributes.timeout
         }, parsedUrl);
-        const protocol = options.protocol === 'https:' ? https : http;
+        const { protocol: reqProtocol, timeout } = options;
+        const protocol = reqProtocol === 'https:' ? https : http;
         const fragmentRequest = protocol.request(options);
-        if (options.timeout) {
-            fragmentRequest.setTimeout(options.timeout, () => fragmentRequest.abort());
+        if (timeout) {
+            fragmentRequest.setTimeout(timeout, fragmentRequest.abort);
         }
         fragmentRequest.on('response', (response) => {
             if (response.statusCode >= 500) {
@@ -42,7 +43,6 @@ module.exports = function requestFragment (fragmentUrl, fragmentAttributes, requ
                 resolve(response);
             }
         });
-        fragmentRequest.on('error', (e) => reject(e));
+        fragmentRequest.on('error', reject);
         fragmentRequest.end();
-    });
-};
+    });
diff --git a/lib/request-handler.js b/lib/request-handler.js
@@ -4,7 +4,7 @@ const Fragment = require('./fragment');
 const StringifierStream = require('./streams/stringifier-stream');
 const ContentLengthStream = require('./streams/content-length-stream');
 const FRAGMENT_EVENTS = ['start', 'response', 'end', 'error', 'timeout', 'fallback', 'warn'];
-const TEMPLATE_NOT_FOUND = require('./fetch-template').TEMPLATE_NOT_FOUND;
+const { TEMPLATE_NOT_FOUND } = require('./fetch-template');
 
 /**
  * Process the HTTP Request to the Tailor Middleware
@@ -16,14 +16,10 @@ const TEMPLATE_NOT_FOUND = require('./fetch-template').TEMPLATE_NOT_FOUND;
 module.exports = function processRequest (options, request, response) {
     this.emit('start', request);
 
-    const fetchContext = options.fetchContext;
-    const fetchTemplate = options.fetchTemplate;
-    const handleTag = options.handleTag;
-    const parseTemplate = options.parseTemplate;
-    const requestFragment = options.requestFragment;
-    const pipeInstanceName = options.pipeInstanceName();
-    const pipeDefinition = options.pipeDefinition(pipeInstanceName);
-    const pipeAttributes = options.pipeAttributes;
+    const { fetchContext, fetchTemplate, handleTag, 
+            parseTemplate, requestFragment, fragmentTag,
+            pipeAttributes, pipeInstanceName, pipeDefinition} = options;
+    const pipeName = pipeInstanceName();
 
     const asyncStream = new AsyncStream();
     const contextPromise = fetchContext(request).catch((err) => {
@@ -48,23 +44,23 @@ module.exports = function processRequest (options, request, response) {
         });
 
         const resultStream = new StringifierStream((tag) => {
-
-            if (tag.placeholder === 'pipe') {
-                return pipeDefinition;
+            const { placeholder, name, } = tag;
+            if (placeholder === 'pipe') {
+                return pipeDefinition(pipeName);
             }
 
-            if (tag.placeholder === 'async') {
+            if (placeholder === 'async') {
                 // end of body tag
                 return asyncStream;
             }
 
-            if (tag.name === options.fragmentTag) {
+            if (name === fragmentTag) {
                 const fragment = new Fragment(
                     tag,
                     context,
                     index++,
                     requestFragment,
-                    pipeInstanceName,
+                    pipeName,
                     pipeAttributes
                 );
 
@@ -75,11 +71,13 @@ module.exports = function processRequest (options, request, response) {
                     });
                 });
 
-                if (fragment.attributes.async) {
-                    asyncStream.write(fragment.stream);
+                const { attributes: { async, primary }, stream} = fragment;
+
+                if (async) {
+                    asyncStream.write(stream);
                 }
 
-                if (fragment.attributes.primary && shouldWriteHead) {
+                if (primary && shouldWriteHead) {
                     shouldWriteHead = false;
                     fragment.on('response', (statusCode, headers) => {
                         if (headers.location) {

diff --git a/lib/serializer.js b/lib/serializer.js
@@ -55,7 +55,7 @@ module.exports = class CustomSerializer extends Serializer {
      * @returns {Boolean}
      */
     _isPipeNode(node) {
-        return this.pipeTags.indexOf(node.name) !== -1;
+        return this.pipeTags.includes(node.name);
     }
 
     /**
@@ -64,12 +64,9 @@ module.exports = class CustomSerializer extends Serializer {
      * @returns {Boolean}
      */
     _isSlotNode(node) {
-        if (node.name === 'slot') {
-            return true;
-        }
-        const attribs = node.attribs;
-        return node.name === 'script' &&
-            attribs && attribs.type === 'slot';
+        const { attribs, name } = node; 
+        return (name === 'slot') ||
+            (name === 'script' && attribs && attribs.type === 'slot');
     }
 
     /**
@@ -78,14 +75,10 @@ module.exports = class CustomSerializer extends Serializer {
      * @returns {Boolean}
      */
     _isSpecialNode(node) {
-        if (this.handleTags.indexOf(node.name) !== -1) {
-            return true;
-        }
-        const attribs = node.attribs;
-        if (attribs && attribs.type) {
-            return node.name === 'script' && this.handleTags.indexOf(attribs.type) !== -1;
-        }
-        return false;
+        const { attribs, name } = node;
+        return this.handleTags.includes(name) ||
+            !!(attribs && attribs.type && name === 'script' 
+                && this.handleTags.includes(attribs.type));
     }
 
     /**
@@ -94,13 +87,8 @@ module.exports = class CustomSerializer extends Serializer {
      * @returns {Boolean}
      */
     _isLastChildOfBody(node) {
-        const parentNode = node.parent;
-        if (parentNode.name === 'body') {
-            if (Object.is(node, parentNode.lastChild)) {
-                return true;
-            }
-        }
-        return false;
+        const { parentNode: { name, lastChild } } = node;
+        return !!(name === 'body' && Object.is(node, lastChild));
     }
 
     /**
@@ -122,16 +110,13 @@ module.exports = class CustomSerializer extends Serializer {
      */
     _serializeSpecial(node) {
         this.pushBuffer();
-        let fragmentObj;
-        if (this.handleTags.indexOf(node.name) !== -1) {
-            fragmentObj = Object.assign({}, { name: node.name, attributes: node.attribs});
-        } else {
-            fragmentObj = Object.assign({}, { name: node.attribs.type, attributes: node.attribs});
-        }
+        const { name: nodeName, attribs: attributes } = node;
+        const name = this.handleTags.includes(nodeName) ? nodeName : attributes.type;
+        const fragmentObj = Object.assign({}, { name, attributes });
         this.serializedList.push(fragmentObj);
         this._serializeChildNodes(node);
         this.pushBuffer();
-        this.serializedList.push({ closingTag: node.name });
+        this.serializedList.push({ closingTag: nodeName });
     }
 
     /**
@@ -143,10 +128,7 @@ module.exports = class CustomSerializer extends Serializer {
         const slotName = node.attribs.name;
         if (slotName) {
             const childNodes = this.treeAdapter.getChildNodes(node);
-            let slots = childNodes;
-            if (this.slotMap.has(slotName)) {
-                slots = this.slotMap.get(slotName);
-            }
+            const slots = this.slotMap.has(slotName) ? this.slotMap.get(slotName) : childNodes;
             slots && slots.forEach(this._serializeNode);
         } else {
             // Handling duplicate slots
@@ -194,9 +176,7 @@ module.exports = class CustomSerializer extends Serializer {
      */
     _serializeChildNodes(parentNode) {
         const childNodes = this.treeAdapter.getChildNodes(parentNode);
-        if (childNodes) {
-            childNodes.forEach(this._serializeNode);
-        }
+        childNodes && childNodes.forEach(this._serializeNode);
     }
 
     /**