fix issue with preloading wrong assets on pages (#141)

* fix issue with preloading wrong assets on pages * change to const
zalando · Apr 12, 2017 · afbc5ea · afbc5ea
1 parent f6114fe
commit afbc5ea
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 30 deletions.
diff --git a/lib/request-handler.js b/lib/request-handler.js
@@ -13,6 +13,24 @@ const getCrossOriginHeader = (fragmentUrl, host) => {
     return '';
 };
 
+// Early preloading of primary fragments assets to improve Performance
+const getAssetsToPreload = ({ link }, { headers = {} }) => {
+    let assetsToPreload = '';
+    const links = parseLinkHeader(link);
+    // Handle Server rendered fragments without depending on assets
+    if (links == null) {
+        return assetsToPreload;
+    }
+    if (links.stylesheet && links.stylesheet.url) {
+        assetsToPreload += `<${links.stylesheet.url}>; rel="preload"; as="style"; nopush,`;
+    }
+    if (links['fragment-script'] && links['fragment-script'].url) {
+        const crossOrigin = getCrossOriginHeader(links['fragment-script'].url, headers.host);
+        assetsToPreload += `<${links['fragment-script'].url}>; rel="preload"; as="script"; nopush; ${crossOrigin}`;
+    }
+    return assetsToPreload;
+};
+
 /**
  * Process the HTTP Request to the Tailor Middleware
  *
@@ -42,33 +60,6 @@ module.exports = function processRequest (options, request, response) {
 
     let shouldWriteHead = true;
     let index = 0;
-    let assetsToPreload = '';
-
-    // Early preloading in browsers to improve the primary fragments
-    const getAssetsToPreload = (fragmentReq, fragment, statusCode, headers) => {
-        // Do not preload assets for non primary fragments
-        if (!fragment.primary) {
-            return;
-        }
-        const links = parseLinkHeader(headers.link);
-        // Remove the event listener once we are done capturing the necessary assets
-        // to make sure we are not exceeding EventListener's limit (10 default)
-        this.removeListener('fragment:response', getAssetsToPreload);
-
-        // Handle Server rendered fragments without depending on assets
-        if (links == null) {
-            return;
-        }
-        if (links.stylesheet && links.stylesheet.url) {
-            assetsToPreload += `<${links.stylesheet.url}>; rel="preload"; as="style"; nopush,`;
-        }
-        if (links['fragment-script'] && links['fragment-script'].url) {
-            const crossOrigin = getCrossOriginHeader(links['fragment-script'].url, request.headers.host);
-            assetsToPreload += `<${links['fragment-script'].url}>; rel="preload"; as="script"; nopush; ${crossOrigin},`;
-        }
-    };
-
-    this.on('fragment:response', getAssetsToPreload);
 
     contextPromise.then((context) => {
 
@@ -118,7 +109,10 @@ module.exports = function processRequest (options, request, response) {
                         }
                         this.emit('response', request, statusCode, responseHeaders);
                         // Make resources early discoverable while processing HTML
-                        assetsToPreload && response.setHeader('Link', assetsToPreload);
+                        const preloadAssets = getAssetsToPreload(headers, request);
+                        if (preloadAssets !== '') {
+                            response.setHeader('Link', preloadAssets);
+                        }
                         response.writeHead(statusCode, responseHeaders);
                         resultStream
                             .pipe(contentLengthStream)

diff --git a/tests/tailor.js b/tests/tailor.js
@@ -770,7 +770,7 @@ describe('Tailor', () => {
             );
 
         getResponse('http://localhost:8080/test').then((response) => {
-            assert.equal(response.headers.link, '<http://primary>; rel="preload"; as="style"; nopush,<http://primary>; rel="preload"; as="script"; nopush; crossorigin,');
+            assert.equal(response.headers.link, '<http://primary>; rel="preload"; as="style"; nopush,<http://primary>; rel="preload"; as="script"; nopush; crossorigin');
         }).then(done, done);
     });
 
@@ -783,7 +783,7 @@ describe('Tailor', () => {
         mockTemplate.returns('<fragment primary src="http://fragment/"></fragment>');
 
         getResponse('http://localhost:8080/test').then((response) => {
-            assert.equal(response.headers.link, '<http://localhost:8080>; rel="preload"; as="style"; nopush,<http://localhost:8080>; rel="preload"; as="script"; nopush; ,');
+            assert.equal(response.headers.link, '<http://localhost:8080>; rel="preload"; as="style"; nopush,<http://localhost:8080>; rel="preload"; as="script"; nopush; ');
         }).then(done, done);
     });