(perf): minor optimization on attributes extraction

lib/filter-headers.js

@@ -18,10 +18,10 @@ const ACCEPT_HEADERS = [
  * @returns {Object} New filtered header object
  */
 module.exports = (attributes, request) => {
-    const { public: publicFragment } = attributes;
+    const { public: isPublic } = attributes;
     const { headers = {} } = request;
     // Headers are not forwarded to public fragment for security reasons
-    return publicFragment
+    return isPublic
         ? {}
         : ACCEPT_HEADERS.reduce((newHeaders, key) => {
               headers[key] && (newHeaders[key] = headers[key]);

lib/fragment.js

@@ -9,6 +9,13 @@ const zlib = require('zlib');
 const { globalTracer, Tags } = require('opentracing');
 const tracer = globalTracer();
 
+const hasValue = value => {
+    if (value || value === '') {
+        return true;
+    }
+    return false;
+};
+
 /**
  * Merge the attributes based on the fragment tag attributes and context
  *
@@ -24,12 +31,23 @@ const getAttributes = (tag, context) => {
         Object.assign(attributes, fragmentCtxt);
     }
 
-    return Object.assign({}, attributes, {
-        url: attributes.src,
+    const {
+        src,
+        async: isAsync,
+        primary,
+        public: isPublic,
+        timeout
+    } = attributes;
+
+    return {
+        url: src,
         id: fragmentId,
+        async: hasValue(isAsync),
+        primary: hasValue(primary),
+        public: hasValue(isPublic),
         fallbackUrl: attributes['fallback-src'],
-        timeout: parseInt(attributes.timeout || 3000, 10)
-    });
+        timeout: parseInt(timeout || 3000, 10)
+    };
 };
 
 /**
@@ -57,23 +75,12 @@ module.exports = class Fragment extends EventEmitter {
         } = {}
     ) {
         super();
-        this.attributes = getAttributes(tag, context, index);
-        ['async', 'primary', 'public'].forEach(key => {
-            let value = this.attributes[key];
-            if (value || value === '') {
-                value = true;
-            } else {
-                value = false;
-            }
-            this.attributes[key] = value;
-        });
-
+        this.attributes = getAttributes(tag, context);
         this.index = index;
         this.maxAssetLinks = maxAssetLinks;
-        this.getPipeAttributes = () =>
-            pipeAttributes(
-                Object.assign({}, { id: this.index }, tag.attributes)
-            );
+        this.pipeAttributes = pipeAttributes(
+            Object.assign({}, { id: this.index }, tag.attributes)
+        );
         this.requestFragment = requestFragment;
         this.pipeInstanceName = pipeInstanceName;
         this.stream = new PassThrough();
@@ -96,22 +103,29 @@ module.exports = class Fragment extends EventEmitter {
 
         const spanOptions = parentSpan ? { childOf: parentSpan } : {};
         const span = tracer.startSpan('fetch-fragment', spanOptions);
+
+        const {
+            id = 'unnamed',
+            primary,
+            async: isAsync,
+            public: isPublic
+        } = this.attributes;
+
         span.addTags({
             [Tags.SPAN_KIND]: Tags.SPAN_KIND_RPC_CLIENT,
             [Tags.HTTP_URL]: url,
-            id: this.attributes.id || 'unnamed',
             fallback: isFallback,
-            primary: this.attributes.primary,
-            async: this.attributes.async,
-            public: this.attributes.public
+            public: isPublic,
+            async: isAsync,
+            id,
+            primary
         });
 
         this.requestFragment(url, this.attributes, request, span).then(
             res => this.onResponse(res, isFallback, span),
             err => {
                 if (!isFallback) {
-                    const { fallbackUrl } = this.attributes;
-                    if (fallbackUrl) {
+                    if (this.attributes.fallbackUrl) {
                         this.emit('fallback', err);
                         this.fetch(request, true, span);
                     } else {
@@ -133,7 +147,7 @@ module.exports = class Fragment extends EventEmitter {
             }
         );
         // Async fragments are piped later on the page
-        if (this.attributes.async) {
+        if (isAsync) {
             return Buffer.from(
                 `<script data-pipe>${this.pipeInstanceName}.placeholder(${this
                     .index})</script>`
@@ -215,9 +229,10 @@ module.exports = class Fragment extends EventEmitter {
      * - CSS for the async fragments are loaded using custom loadCSS(available in src/pipe.js)
      */
     insertStart() {
+        const { async: isAsync, id } = this.attributes;
         this.styleRefs.forEach(uri => {
             this.stream.write(
-                this.attributes.async
+                isAsync
                     ? `<script>${this
                           .pipeInstanceName}.loadCSS("${uri}")</script>`
                     : `<link rel="stylesheet" href="${uri}">`
@@ -234,12 +249,12 @@ module.exports = class Fragment extends EventEmitter {
         }
 
         const range = [this.index, this.index + this.scriptRefs.length - 1];
-        const fragmentId = this.attributes.id || range[0];
+        const fragmentId = id || range[0];
+        const attributes = Object.assign({}, this.pipeAttributes, {
+            id: fragmentId,
+            range
+        });
         this.scriptRefs.forEach(uri => {
-            const attributes = Object.assign({}, this.getPipeAttributes(), {
-                id: fragmentId,
-                range
-            });
             this.stream.write(
                 `<script data-pipe>${this.pipeInstanceName}.start(${this
                     .index}, "${uri}", ${JSON.stringify(attributes)})</script>`
@@ -256,11 +271,11 @@ module.exports = class Fragment extends EventEmitter {
             const range = [this.index - this.scriptRefs.length, this.index - 1];
             this.index--;
             const fragmentId = this.attributes.id || range[0];
+            const attributes = Object.assign({}, this.pipeAttributes, {
+                id: fragmentId,
+                range
+            });
             this.scriptRefs.reverse().forEach(uri => {
-                const attributes = Object.assign({}, this.getPipeAttributes(), {
-                    id: fragmentId,
-                    range
-                });
                 this.stream.write(
                     `<script data-pipe>${this.pipeInstanceName}.end(${this
                         .index--}, "${uri}", ${JSON.stringify(

lib/request-handler.js

@@ -81,11 +81,9 @@ module.exports = function processRequest(options, request, response) {
 
     const handleError = err => {
         this.emit('error', request, err);
+        const { message, stack } = err;
         span.setTag(Tags.ERROR, true);
-        span.log({
-            message: err.message,
-            stack: err.stack
-        });
+        span.log({ message, stack });
         if (shouldWriteHead) {
             shouldWriteHead = false;
             let statusCode = 500;
@@ -185,20 +183,16 @@ module.exports = function processRequest(options, request, response) {
 
             resultStream.on('fragment:found', fragment => {
                 isFragmentFound = true;
+                const { attributes } = fragment;
                 FRAGMENT_EVENTS.forEach(eventName => {
                     fragment.once(eventName, (...args) => {
                         const prefixedName = 'fragment:' + eventName;
-                        this.emit(
-                            prefixedName,
-                            request,
-                            fragment.attributes,
-                            ...args
-                        );
+                        this.emit(prefixedName, request, attributes, ...args);
                     });
                 });
 
-                const { primary } = fragment.attributes;
-                primary && handlePrimaryFragment(fragment, resultStream);
+                attributes.primary &&
+                    handlePrimaryFragment(fragment, resultStream);
             });
 
             resultStream.once('finish', () => {

tests/fragment.js

@@ -12,7 +12,7 @@ const getOptions = tag => {
 };
 
 describe('Fragment', () => {
-    it('computed and custom attributes are correctly initiliazed', () => {
+    it('computed attributes are correctly initiliazed', () => {
         const attributes = {
             id: 'foo',
             src: 'https://fragment',
@@ -25,14 +25,11 @@ describe('Fragment', () => {
         const expected = {
             id: attributes.id,
             url: attributes.src,
-            'fallback-src': attributes['fallback-src'],
-            src: attributes.src,
             fallbackUrl: attributes['fallback-src'],
             async: attributes.async,
             timeout: 4000,
             primary: false,
-            public: false,
-            custom: attributes.custom
+            public: false
         };
 
         const tag = { attributes };