From 925bada3983b91eb49ec91a98845e30f6de1dde9 Mon Sep 17 00:00:00 2001 From: iilei Date: Thu, 25 May 2017 12:37:00 +0200 Subject: [PATCH 1/2] Fix hooks * Add tests for Hooks execution order and amount * follow-up on #140 --- .../fragment.js | 27 +++-- .../index.js | 8 +- .../templates/base-template.html | 40 ++++++- .../test.js | 27 +++-- index.js | 1 - lib/fragment.js | 14 +-- src/pipe.js | 83 ++++++++++++-- tests/tailor.js | 102 +++++++++--------- 8 files changed, 212 insertions(+), 90 deletions(-) diff --git a/examples/multiple-fragments-with-custom-amd/fragment.js b/examples/multiple-fragments-with-custom-amd/fragment.js index 3943539..e4540a8 100644 --- a/examples/multiple-fragments-with-custom-amd/fragment.js +++ b/examples/multiple-fragments-with-custom-amd/fragment.js @@ -10,24 +10,35 @@ const defineFn = (module, fragmentName) => { return `define (['${module}'], function (module) { return function initFragment (element) { element.className += ' fragment-${fragmentName}-${module}'; - element.innerHTML += module; + element.innerHTML += ' ' + module; } })`; }; -module.exports = (fragmentName, fragmentUrl) => (request, response) => { +module.exports = (fragmentName, fragmentUrl, modules=2) => (request, response) => { const pathname = url.parse(request.url).pathname; + const moduleLinks = []; + + for (var i=0; i; rel="fragment-script"`; + } + switch (pathname) { - case '/fragment-1.js': + case '/module-1.js': // serve fragment's JavaScript response.writeHead(200, jsHeaders); response.end(defineFn('js1', fragmentName)); break; - case '/fragment-2.js': + case '/module-2.js': // serve fragment's JavaScript response.writeHead(200, jsHeaders); response.end(defineFn('js2', fragmentName)); break; + case '/module-3.js': + // serve fragment's JavaScript + response.writeHead(200, jsHeaders); + response.end(defineFn('js3', fragmentName)); + break; case '/fragment.css': // serve fragment's CSS response.writeHead(200, { 'Content-Type': 'text/css' }); @@ -43,14 +54,15 @@ module.exports = (fragmentName, fragmentUrl) => (request, response) => { .fragment-${fragmentName}-js2 { color: blue; } + .fragment-${fragmentName}-js3 { + text-decoration: underline; + } `); break; default: // serve fragment's body response.writeHead(200, { - 'Link': `<${fragmentUrl}/fragment.css>; rel="stylesheet",` + - `<${fragmentUrl}/fragment-1.js>; rel="fragment-script",` + - `<${fragmentUrl}/fragment-2.js>; rel="fragment-script"`, + 'Link': `<${fragmentUrl}/fragment.css>; rel="stylesheet",${moduleLinks.join(',')}`, 'Content-Type': 'text/html' }); response.end(` @@ -58,5 +70,6 @@ module.exports = (fragmentName, fragmentUrl) => (request, response) => { Fragment ${fragmentName} `); + } }; diff --git a/examples/multiple-fragments-with-custom-amd/index.js b/examples/multiple-fragments-with-custom-amd/index.js index adc7a8c..a08017a 100644 --- a/examples/multiple-fragments-with-custom-amd/index.js +++ b/examples/multiple-fragments-with-custom-amd/index.js @@ -8,7 +8,7 @@ const baseTemplateFn = () => 'base-template'; const AMD_LOADER = 'file://' + require.resolve('iamdee'); const tailor = new Tailor({ amdLoaderUrl: AMD_LOADER, - maxAssetLinks: 2, + maxAssetLinks: 3, fetchTemplate: fetchTemplateFs(path.join(__dirname, 'templates'), baseTemplateFn) }); const server = http.createServer((req, res) => { @@ -23,19 +23,19 @@ console.log('Tailor started at port 8080'); tailor.on('error', (request, err) => console.error(err)); const fragment1 = http.createServer( - serveFragment('hello', 'http://localhost:8081') + serveFragment('fragment1', 'http://localhost:8081') ); fragment1.listen(8081); console.log('Fragment1 started at port 8081'); const fragment2 = http.createServer( - serveFragment('world', 'http://localhost:8082') + serveFragment('fragment2', 'http://localhost:8082') ); fragment2.listen(8082); console.log('Fragment2 started at port 8082'); const fragment3 = http.createServer( - serveFragment('body-start', 'http://localhost:8083') + serveFragment('fragment3', 'http://localhost:8083', 3) ); fragment3.listen(8083); console.log('Fragment3 started at port 8083'); diff --git a/examples/multiple-fragments-with-custom-amd/templates/base-template.html b/examples/multiple-fragments-with-custom-amd/templates/base-template.html index 171a878..6339fa7 100644 --- a/examples/multiple-fragments-with-custom-amd/templates/base-template.html +++ b/examples/multiple-fragments-with-custom-amd/templates/base-template.html @@ -11,7 +11,42 @@ return 'js1'; }); define('js2', function () { - return '-js2'; + return 'js2'; + }); + define('js3', function () { + return 'js3'; + }); + // Testing hook calls via logs + var log = { + 'fragment-0' : [], + 'fragment-custom-id' : [], + 'fragment-6' : [], + 'common': [], + }; + var fragmentIds = [0, 'custom-id', 6]; + var result = ''; + + Pipe.onStart((attributes) => { + log['fragment-' + attributes.id].push('onStart'); + }); + Pipe.onBeforeInit((attributes) => { + log['fragment-' + attributes.id].push('onBeforeInit'); + }); + Pipe.onAfterInit((attributes) => { + log['fragment-' + attributes.id].push('onAfterInit'); + }); + + Pipe.onDone(() => { + log.common.push('onDone'); + + var logNode = document.getElementById('hook-logs'); + fragmentIds.forEach(function(id, index) { + var key = 'fragment-' + id; + result += 'fragment-' + index + ' hooks: ' + log[key].join(',') +';\n'; + }); + result += ' common hooks: ' + log.common.join(',') + ';'; + logNode.innerText = result; + logNode.className += ' all-done'; }); @@ -24,10 +59,11 @@ document.body.appendChild(document.createElement('script'));

Fragment 1:

- +

Fragment 2:

All done!
+

 
 
diff --git a/examples/multiple-fragments-with-custom-amd/test.js b/examples/multiple-fragments-with-custom-amd/test.js
index 589e053..5073576 100644
--- a/examples/multiple-fragments-with-custom-amd/test.js
+++ b/examples/multiple-fragments-with-custom-amd/test.js
@@ -50,11 +50,15 @@ describe('Frontend test', function () {
             );
     }
 
+    function logForFragment(id) {
+        return ('fragment-' + id +' hooks: onStart,onBeforeInit,onAfterInit;');
+    }
+
     before(() => {
         server = http.createServer(tailor.requestHandler);
-        fragment1 = http.createServer(fragment('hello', 'http://localhost:8081'));
-        fragment2 = http.createServer(fragment('world', 'http://localhost:8082'));
-        fragment3 = http.createServer(fragment('body-start', 'http://localhost:8083'));
+        fragment1 = http.createServer(fragment('fragment1', 'http://localhost:8081'));
+        fragment2 = http.createServer(fragment('fragment2', 'http://localhost:8082'));
+        fragment3 = http.createServer(fragment('fragment3', 'http://localhost:8083'));
         return Promise.all([
             server.listen(8080),
             fragment1.listen(8081),
@@ -81,12 +85,17 @@ describe('Frontend test', function () {
                     .then((title) => {
                         assert.equal(title, 'Test Page', 'Test page is not loaded');
                     })
-                    .waitForElementByCss('.fragment-hello-js1', asserters.textInclude('js1'), 2000)
-                    .waitForElementByCss('.fragment-hello-js2', asserters.textInclude('js2'), 2000)
-                    .waitForElementByCss('.fragment-world-js1', asserters.textInclude('js1'), 2000)
-                    .waitForElementByCss('.fragment-world-js2', asserters.textInclude('js2'), 2000)
-                    .waitForElementByCss('.fragment-body-start-js1', asserters.textInclude('js1'), 2000)
-                    .waitForElementByCss('.fragment-body-start-js2', asserters.textInclude('js2'), 2000);
+                    .waitForElementByCss('.fragment-fragment1-js1', asserters.textInclude('js1'), 2000)
+                    .waitForElementByCss('.fragment-fragment1-js2', asserters.textInclude('js2'), 2000)
+                    .waitForElementByCss('.fragment-fragment2-js1', asserters.textInclude('js1'), 2000)
+                    .waitForElementByCss('.fragment-fragment2-js2', asserters.textInclude('js2'), 2000)
+                    .waitForElementByCss('.fragment-fragment3-js1', asserters.textInclude('js1'), 2000)
+                    .waitForElementByCss('.fragment-fragment3-js2', asserters.textInclude('js2'), 2000)
+                    .waitForElementByCss('.fragment-fragment3-js3', asserters.textInclude('js3'), 2000)
+                    .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(0)), 2000)
+                    .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(1)), 2000)
+                    .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(2)), 2000)
+                    .waitForElementByCss('.logs.all-done', asserters.textInclude('common hooks: onDone;'), 2000);
 
             });
         });
diff --git a/index.js b/index.js
index 385e5be..478a930 100644
--- a/index.js
+++ b/index.js
@@ -11,7 +11,6 @@ const filterHeadersFn = require('./lib/filter-headers');
 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, id } = attributes;
diff --git a/lib/fragment.js b/lib/fragment.js
index 82b1f70..581bb1d 100644
--- a/lib/fragment.js
+++ b/lib/fragment.js
@@ -171,8 +171,8 @@ module.exports = class Fragment extends EventEmitter {
     insertStart () {
         this.styleRefs.forEach(uri => {
             this.stream.write(
-                this.attributes.async 
-                    ? `` 
+                this.attributes.async
+                    ? ``
                     : ``
             );
         });
@@ -183,9 +183,10 @@ module.exports = class Fragment extends EventEmitter {
             return;
         }
 
+        const range = [this.index, this.index + this.scriptRefs.length - 1];
         this.scriptRefs.forEach((uri)=> {
             const id = this.index;
-            const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id });
+            const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id, range });
             this.stream.write(
                 ``
             );
@@ -197,17 +198,18 @@ module.exports = class Fragment extends EventEmitter {
      * Insert the placeholder for pipe assets at the end of fragment stream
      */
     insertEnd () {
-        this.index--;
         if (this.scriptRefs.length > 0) {
+            const range = [this.index - this.scriptRefs.length, this.index - 1];
+            this.index--;
             this.scriptRefs.reverse().forEach((uri)=> {
                 const id = this.index--;
-                const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id });
+                const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id, range });
                 this.stream.write(
                     ``
                 );
             });
         } else {
-            this.stream.write(``);
+            this.stream.write(``);
         }
     }
 
diff --git a/src/pipe.js b/src/pipe.js
index 13d8afe..109a2c5 100644
--- a/src/pipe.js
+++ b/src/pipe.js
@@ -3,7 +3,7 @@
     var starts = {};
     var scripts = doc.getElementsByTagName('script');
     // State to maintain if all fragments on the page are initialized
-    var initState = [];
+    var initState = {};
     var noop = function() {};
     // Hooks that will be replaced later on the page
     var hooks = {
@@ -24,6 +24,69 @@
         }
     }
 
+    function arrayMath(array, mathFn) {
+        return Math[mathFn].apply(null, array);
+    }
+
+    function arrayFlatten(array) {
+        return [].concat.apply([], array);
+    }
+
+    function objectValues(obj) {
+        var keys = Object.keys(obj);
+        var result = [];
+        keys.forEach(function(key) {
+            result.push(obj[key]);
+        });
+        return result;
+    }
+
+    function arrayOf(val, size) {
+        var result = [];
+        for (var i = size; i--; i >= 0) {
+            result[i] = val;
+        }
+        return result;
+    }
+
+    function chunksCount(range) {
+        return (range[1] - range[0] + 1);
+    }
+
+    // setChunkInitPhase keeps track of initialization phases
+    function setChunkInitPhase(index, attributes, phase) {
+        var chunkIndex = index - attributes.range[0];
+        var fragmentId = typeof attributes.id === 'string' ? attributes.id : attributes.range[0];
+        initState[fragmentId][chunkIndex] = phase;
+    }
+
+    function chunkHook(attributes, hook) {
+        var fragmentId = typeof attributes.id === 'string' ? attributes.id : attributes.range[0];
+
+        switch (hook) {
+            case 'onStart':
+                if (!initState[fragmentId]) {
+                    initState[fragmentId] = arrayOf(0, chunksCount(attributes.range));
+                    hooks.onStart(attributes);
+                }
+                break;
+            case 'onBeforeInit':
+                var chunkMax = arrayMath(initState[fragmentId], 'max');
+                chunkMax < 2 && hooks.onBeforeInit(attributes);
+                break;
+            case 'onAfterInit':
+                var chunkMax = arrayMath(initState[fragmentId], 'max');
+                chunkMax < 3 && hooks.onAfterInit(attributes);
+                break;
+            case 'onDone':
+                var allInitStates = arrayFlatten(objectValues(initState));
+                var allChunksMin = arrayMath(allInitStates, 'min');
+                allChunksMin > 2 && hooks.onDone();
+                break;
+
+        }
+    }
+
     function placeholder(index) {
         placeholders[index] = currentScript();
     }
@@ -31,8 +94,8 @@
     function start(index, script, attributes) {
         starts[index] = currentScript();
         if (script) {
-            initState.push(index);
-            hooks.onStart(attributes);
+            chunkHook(attributes, 'onStart');
+            setChunkInitPhase(index, attributes, 1);
             require([script]);
         }
     }
@@ -76,17 +139,17 @@
             }
 
             function doInit(init, node) {
-                hooks.onBeforeInit(attributes);
                 var fragmentRendering = init(node);
+                chunkHook(attributes, 'onBeforeInit');
+                setChunkInitPhase(index, attributes, 2);
                 var handlerFn = function() {
-                    initState.pop();
-                    hooks.onAfterInit(attributes);
+                    chunkHook(attributes, 'onAfterInit');
+                    setChunkInitPhase(index, attributes, 3);
                     // OnDone will be called once the document is completed parsed and there are no other fragments getting streamed.
-                    if (initState.length === 0
-                        && doc.readyState
+                    if (doc.readyState
                         && (doc.readyState === 'complete'
-                        || doc.readyState === 'interactive') ) {
-                        hooks.onDone();
+                        || doc.readyState === 'interactive')) {
+                        chunkHook(attributes, 'onDone');
                     }
                 };
                 // Check if the response from fragment is a Promise to allow lazy rendering
diff --git a/tests/tailor.js b/tests/tailor.js
index d939ec5..236e51f 100644
--- a/tests/tailor.js
+++ b/tests/tailor.js
@@ -74,7 +74,7 @@ describe('Tailor', () => {
             mockTemplate.returns(false);
             getResponse('http://localhost:8080/missing-template').then((response) => {
                 assert.equal(response.statusCode, 500);
-                assert.equal(response.body, '
error template
'); + assert.equal(response.body, '
error template
'); }).then(done, done); }); @@ -269,7 +269,7 @@ describe('Tailor', () => { }).then(done, done); }); }); - + describe('Timeout::Tailor ', () => { it('should set timeout for a fragment request', (done) => { nock('https://fragment') @@ -369,9 +369,9 @@ describe('Tailor', () => { '' + '' + '' + - '' + + '' + 'hello' + - '' + + '' + '' + '' ); @@ -392,9 +392,9 @@ describe('Tailor', () => { '' + '' + '' + - '' + + '' + 'hello' + - '' + + '' + '' ); }).then(done, done); @@ -415,16 +415,16 @@ describe('Tailor', () => { '' + '' + '' + - '' + + '' + 'hello' + - '' + + '' + '' + '' ); }).then(done, done); }); }); - + describe('Attributes and Context::Tailor', () => { it('should call the pipe start and end with custom pipe attributes', (done) => { nock('https://fragment') @@ -438,9 +438,9 @@ describe('Tailor', () => { getResponse('http://localhost:8080/test').then((response) => { assert.equal(response.body, '' + - '' + + '' + 'hello' + - '' + + '' + '' ); }).then(done, done); @@ -717,7 +717,7 @@ describe('Tailor', () => { }); }); }); - + describe('Zip::Tailor ', () => { it('should unzip the fragment response if it is compressed', (done) => { nock('https://fragment') @@ -795,9 +795,9 @@ describe('Tailor', () => { getResponse('http://localhost:8080/test').then((response) => { assert.equal(response.body, '' + - '' + + '' + 'hello maxAssetLinks default' + - '' + + '' + '' ); }).then(done, done); @@ -855,13 +855,13 @@ describe('Tailor', () => { getResponse('http://localhost:8081/test').then((response) => { assert.equal(response.body, '' + - '' + - '' + - '' + + '' + + '' + + '' + 'hello multiple' + - '' + - '' + - '' + + '' + + '' + + '' + '' ); }).then(done, done); @@ -880,13 +880,13 @@ describe('Tailor', () => { getResponse('http://localhost:8081/test').then((response) => { assert.equal(response.body, '' + - '' + - '' + - '' + + '' + + '' + + '' + 'hello multiple' + - '' + - '' + - '' + + '' + + '' + + '' + '' ); }).then(done, done); @@ -911,7 +911,7 @@ describe('Tailor', () => { mockTemplate .returns( '' + - '' + + '' + '' + '' ); @@ -919,37 +919,37 @@ describe('Tailor', () => { getResponse('http://localhost:8081/test').then((response) => { assert.equal(response.body, '' + - '' + - '' + - '' + + '' + + '' + + '' + 'hello many' + - '' + - '' + - '' + + '' + + '' + + '' + '' + '' + - '' + - '' + - '' + + '' + + '' + + '' + 'hello exactly three' + - '' + - '' + - '' + + '' + + '' + + '' + - '' + + '' + 'hello single' + - '' + + '' + - '' + - '' + - '' + + '' + + '' + + '' + 'hello exactly three async' + - '' + - '' + - '' + + '' + + '' + + '' + '' ); @@ -972,9 +972,9 @@ describe('Tailor', () => { '' + '' + '' + - '' + + '' + 'hello multiple styles ' + - '' + + '' + '' + '' ); @@ -997,9 +997,9 @@ describe('Tailor', () => { '' + '' + '' + - '' + + '' + 'hello multiple styles async' + - '' + + '' + '' ); }).then(done, done); From 4892792fda3d63d4ded8375b97c60004e166b2fc Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 12 Jun 2017 12:18:39 +0200 Subject: [PATCH 2/2] change the hooks logic for fragment --- docs/hooks.md | 19 ++-- .../fragment-performance/templates/index.html | 1 - .../fragment.js | 10 +- .../index.js | 4 +- .../templates/base-template.html | 22 ++--- .../test.js | 19 ++-- lib/fragment.js | 12 +-- src/pipe.js | 97 ++++--------------- tests/tailor.js | 56 +++-------- 9 files changed, 78 insertions(+), 162 deletions(-) diff --git a/docs/hooks.md b/docs/hooks.md index c523688..5c910f1 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -4,17 +4,17 @@ Tailor provides four hooks on the client side(Browser) that can be used for prog ## API -### Pipe.onStart(callback(attributes)) -+ callback will be called before every fragments gets inserted/piped in the browser. +### Pipe.onStart(callback(attributes, index)) ++ callback will be called before every script from fragments gets inserted/piped in the browser. -### Pipe.onBeforeInit(callback(attributes)) -+ callback will be called before every fragments on the page/template gets initialized. +### Pipe.onBeforeInit(callback(attributes, index)) ++ callback will be called before each script from fragments on the page/template gets initialized. -### Pipe.onAfterInit(callback(attributes)) -+ callback will be called after each fragments on the page gets initialized. +### Pipe.onAfterInit(callback(attributes, index)) ++ callback will be called after each script from fragments on the page gets initialized. ### Pipe.onDone(callback()) -+ callback will be called when all the fragments on the page gets initialized. ++ callback will be called when all the fragment scripts on the page gets initialized. ## Options @@ -22,4 +22,9 @@ Tailor provides four hooks on the client side(Browser) that can be used for prog + The attributes that are available from hooks can be customized by overiding `pipeAttributes` function as part of Tailor options. + The default attributes that are available through hooks are `primary` and `id`. +#### index ++ The order in which the script tags(sent via `Link Headers` from each fragment) are flushed to the browser + +**NOTE: Hooks wont work properly for scripts/modules that are not AMD** + Check out the [Performance](https://github.com/zalando/tailor/tree/master/docs/Performance.md) document on how to measure fragment initialzation time as well as capturing the time to interactivity of the page. diff --git a/examples/fragment-performance/templates/index.html b/examples/fragment-performance/templates/index.html index 876d555..bf933bd 100644 --- a/examples/fragment-performance/templates/index.html +++ b/examples/fragment-performance/templates/index.html @@ -56,7 +56,6 @@ diff --git a/examples/multiple-fragments-with-custom-amd/fragment.js b/examples/multiple-fragments-with-custom-amd/fragment.js index e4540a8..31bb2ae 100644 --- a/examples/multiple-fragments-with-custom-amd/fragment.js +++ b/examples/multiple-fragments-with-custom-amd/fragment.js @@ -15,7 +15,7 @@ const defineFn = (module, fragmentName) => { })`; }; -module.exports = (fragmentName, fragmentUrl, modules=2) => (request, response) => { +module.exports = (fragmentName, fragmentUrl, modules = 1) => (request, response) => { const pathname = url.parse(request.url).pathname; const moduleLinks = []; @@ -34,11 +34,6 @@ module.exports = (fragmentName, fragmentUrl, modules=2) => (request, response) = response.writeHead(200, jsHeaders); response.end(defineFn('js2', fragmentName)); break; - case '/module-3.js': - // serve fragment's JavaScript - response.writeHead(200, jsHeaders); - response.end(defineFn('js3', fragmentName)); - break; case '/fragment.css': // serve fragment's CSS response.writeHead(200, { 'Content-Type': 'text/css' }); @@ -54,9 +49,6 @@ module.exports = (fragmentName, fragmentUrl, modules=2) => (request, response) = .fragment-${fragmentName}-js2 { color: blue; } - .fragment-${fragmentName}-js3 { - text-decoration: underline; - } `); break; default: diff --git a/examples/multiple-fragments-with-custom-amd/index.js b/examples/multiple-fragments-with-custom-amd/index.js index a08017a..ff5fed9 100644 --- a/examples/multiple-fragments-with-custom-amd/index.js +++ b/examples/multiple-fragments-with-custom-amd/index.js @@ -8,7 +8,7 @@ const baseTemplateFn = () => 'base-template'; const AMD_LOADER = 'file://' + require.resolve('iamdee'); const tailor = new Tailor({ amdLoaderUrl: AMD_LOADER, - maxAssetLinks: 3, + maxAssetLinks: 2, fetchTemplate: fetchTemplateFs(path.join(__dirname, 'templates'), baseTemplateFn) }); const server = http.createServer((req, res) => { @@ -35,7 +35,7 @@ fragment2.listen(8082); console.log('Fragment2 started at port 8082'); const fragment3 = http.createServer( - serveFragment('fragment3', 'http://localhost:8083', 3) + serveFragment('fragment3', 'http://localhost:8083', 2) ); fragment3.listen(8083); console.log('Fragment3 started at port 8083'); diff --git a/examples/multiple-fragments-with-custom-amd/templates/base-template.html b/examples/multiple-fragments-with-custom-amd/templates/base-template.html index 6339fa7..e24bc07 100644 --- a/examples/multiple-fragments-with-custom-amd/templates/base-template.html +++ b/examples/multiple-fragments-with-custom-amd/templates/base-template.html @@ -13,29 +13,25 @@ define('js2', function () { return 'js2'; }); - define('js3', function () { - return 'js3'; - }); // Testing hook calls via logs var log = { 'fragment-0' : [], 'fragment-custom-id' : [], - 'fragment-6' : [], + 'fragment-4' : [], 'common': [], }; - var fragmentIds = [0, 'custom-id', 6]; + var fragmentIds = [0, 'custom-id', 4]; var result = ''; - Pipe.onStart((attributes) => { - log['fragment-' + attributes.id].push('onStart'); + Pipe.onStart((attributes, index) => { + log['fragment-' + attributes.id].push('onStart-' + index); }); - Pipe.onBeforeInit((attributes) => { - log['fragment-' + attributes.id].push('onBeforeInit'); + Pipe.onBeforeInit((attributes, index) => { + log['fragment-' + attributes.id].push('onBeforeInit-' + index); }); - Pipe.onAfterInit((attributes) => { - log['fragment-' + attributes.id].push('onAfterInit'); + Pipe.onAfterInit((attributes, index) => { + log['fragment-' + attributes.id].push('onAfterInit-' + index); }); - Pipe.onDone(() => { log.common.push('onDone'); @@ -44,7 +40,7 @@ var key = 'fragment-' + id; result += 'fragment-' + index + ' hooks: ' + log[key].join(',') +';\n'; }); - result += ' common hooks: ' + log.common.join(',') + ';'; + result += ' common hooks: ' + log.common.join(',') +';'; logNode.innerText = result; logNode.className += ' all-done'; }); diff --git a/examples/multiple-fragments-with-custom-amd/test.js b/examples/multiple-fragments-with-custom-amd/test.js index 5073576..ccfad79 100644 --- a/examples/multiple-fragments-with-custom-amd/test.js +++ b/examples/multiple-fragments-with-custom-amd/test.js @@ -51,7 +51,15 @@ describe('Frontend test', function () { } function logForFragment(id) { - return ('fragment-' + id +' hooks: onStart,onBeforeInit,onAfterInit;'); + const result = 'fragment-' + id +' hooks: '; + switch (id) { + case '0': + return result + 'onStart-0,onStart-1,onBeforeInit-0,onAfterInit-0,onBeforeInit-1,onAfterInit-1;'; + case '1': + return result + 'onStart-2,onBeforeInit-2,onAfterInit-2;'; + case '2': + return result + 'onStart-4,onBeforeInit-4,onAfterInit-4;'; + } } before(() => { @@ -86,15 +94,12 @@ describe('Frontend test', function () { assert.equal(title, 'Test Page', 'Test page is not loaded'); }) .waitForElementByCss('.fragment-fragment1-js1', asserters.textInclude('js1'), 2000) - .waitForElementByCss('.fragment-fragment1-js2', asserters.textInclude('js2'), 2000) .waitForElementByCss('.fragment-fragment2-js1', asserters.textInclude('js1'), 2000) - .waitForElementByCss('.fragment-fragment2-js2', asserters.textInclude('js2'), 2000) .waitForElementByCss('.fragment-fragment3-js1', asserters.textInclude('js1'), 2000) .waitForElementByCss('.fragment-fragment3-js2', asserters.textInclude('js2'), 2000) - .waitForElementByCss('.fragment-fragment3-js3', asserters.textInclude('js3'), 2000) - .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(0)), 2000) - .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(1)), 2000) - .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment(2)), 2000) + .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment('0')), 2000) + .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment('1')), 2000) + .waitForElementByCss('.logs.all-done', asserters.textInclude(logForFragment('2')), 2000) .waitForElementByCss('.logs.all-done', asserters.textInclude('common hooks: onDone;'), 2000); }); diff --git a/lib/fragment.js b/lib/fragment.js index 581bb1d..5b0da03 100644 --- a/lib/fragment.js +++ b/lib/fragment.js @@ -184,11 +184,11 @@ module.exports = class Fragment extends EventEmitter { } const range = [this.index, this.index + this.scriptRefs.length - 1]; + const fragmentId = this.attributes.id || range[0]; this.scriptRefs.forEach((uri)=> { - const id = this.index; - const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id, range }); + const attributes = Object.assign({}, this.getPipeAttributes(), { id: fragmentId, range }); this.stream.write( - `` + `` ); this.index++; }); @@ -201,11 +201,11 @@ module.exports = class Fragment extends EventEmitter { if (this.scriptRefs.length > 0) { const range = [this.index - this.scriptRefs.length, this.index - 1]; this.index--; + const fragmentId = this.attributes.id || range[0]; this.scriptRefs.reverse().forEach((uri)=> { - const id = this.index--; - const attributes = Object.assign({}, this.getPipeAttributes(), { id: this.attributes.id || id, range }); + const attributes = Object.assign({}, this.getPipeAttributes(), { id: fragmentId, range }); this.stream.write( - `` + `` ); }); } else { diff --git a/src/pipe.js b/src/pipe.js index 109a2c5..69c21fb 100644 --- a/src/pipe.js +++ b/src/pipe.js @@ -3,7 +3,7 @@ var starts = {}; var scripts = doc.getElementsByTagName('script'); // State to maintain if all fragments on the page are initialized - var initState = {}; + var initState = []; var noop = function() {}; // Hooks that will be replaced later on the page var hooks = { @@ -24,69 +24,6 @@ } } - function arrayMath(array, mathFn) { - return Math[mathFn].apply(null, array); - } - - function arrayFlatten(array) { - return [].concat.apply([], array); - } - - function objectValues(obj) { - var keys = Object.keys(obj); - var result = []; - keys.forEach(function(key) { - result.push(obj[key]); - }); - return result; - } - - function arrayOf(val, size) { - var result = []; - for (var i = size; i--; i >= 0) { - result[i] = val; - } - return result; - } - - function chunksCount(range) { - return (range[1] - range[0] + 1); - } - - // setChunkInitPhase keeps track of initialization phases - function setChunkInitPhase(index, attributes, phase) { - var chunkIndex = index - attributes.range[0]; - var fragmentId = typeof attributes.id === 'string' ? attributes.id : attributes.range[0]; - initState[fragmentId][chunkIndex] = phase; - } - - function chunkHook(attributes, hook) { - var fragmentId = typeof attributes.id === 'string' ? attributes.id : attributes.range[0]; - - switch (hook) { - case 'onStart': - if (!initState[fragmentId]) { - initState[fragmentId] = arrayOf(0, chunksCount(attributes.range)); - hooks.onStart(attributes); - } - break; - case 'onBeforeInit': - var chunkMax = arrayMath(initState[fragmentId], 'max'); - chunkMax < 2 && hooks.onBeforeInit(attributes); - break; - case 'onAfterInit': - var chunkMax = arrayMath(initState[fragmentId], 'max'); - chunkMax < 3 && hooks.onAfterInit(attributes); - break; - case 'onDone': - var allInitStates = arrayFlatten(objectValues(initState)); - var allChunksMin = arrayMath(allInitStates, 'min'); - allChunksMin > 2 && hooks.onDone(); - break; - - } - } - function placeholder(index) { placeholders[index] = currentScript(); } @@ -94,12 +31,22 @@ function start(index, script, attributes) { starts[index] = currentScript(); if (script) { - chunkHook(attributes, 'onStart'); - setChunkInitPhase(index, attributes, 1); + initState.push(index); + hooks.onStart(attributes, index); require([script]); } } + // OnDone will be called once the document is completed parsed and there are no other fragments getting streamed. + function fireDone() { + if (initState.length === 0 + && doc.readyState + && (doc.readyState === 'complete' + || doc.readyState === 'interactive') ) { + hooks.onDone(); + } + } + function end(index, script, attributes) { var placeholder = placeholders[index]; var start = starts[index]; @@ -126,10 +73,12 @@ start.parentNode.removeChild(start); end.parentNode.removeChild(end); script && require([script], function (i) { - // Exposed fragment initialization Function/Promise + // Exported AMD fragment initialization Function/Promise var init = i && i.__esModule ? i.default : i; // early return if (typeof init !== 'function') { + initState.pop(); + fireDone(); return; } @@ -139,18 +88,12 @@ } function doInit(init, node) { + hooks.onBeforeInit(attributes, index); var fragmentRendering = init(node); - chunkHook(attributes, 'onBeforeInit'); - setChunkInitPhase(index, attributes, 2); var handlerFn = function() { - chunkHook(attributes, 'onAfterInit'); - setChunkInitPhase(index, attributes, 3); - // OnDone will be called once the document is completed parsed and there are no other fragments getting streamed. - if (doc.readyState - && (doc.readyState === 'complete' - || doc.readyState === 'interactive')) { - chunkHook(attributes, 'onDone'); - } + initState.pop(); + hooks.onAfterInit(attributes, index); + fireDone(attributes); }; // Check if the response from fragment is a Promise to allow lazy rendering if (isPromise(fragmentRendering)) { diff --git a/tests/tailor.js b/tests/tailor.js index 236e51f..1fb79d4 100644 --- a/tests/tailor.js +++ b/tests/tailor.js @@ -843,30 +843,6 @@ describe('Tailor', () => { serverCustomOptions.close(done); }); - it('should handle all 3 fragment-script Link-rels', (done) => { - nock('https://fragment') - .get('/1').reply(200, 'hello multiple', { - 'Link': '; rel="fragment-script", ; rel="fragment-script", ; rel="fragment-script"' - }); - - mockTemplate - .returns(''); - - getResponse('http://localhost:8081/test').then((response) => { - assert.equal(response.body, - '' + - '' + - '' + - '' + - 'hello multiple' + - '' + - '' + - '' + - '' - ); - }).then(done, done); - }); - it('should handle only the first 3 fragment-script Link-rels', (done) => { nock('https://fragment') .get('/1').reply(200, 'hello multiple', { @@ -881,11 +857,11 @@ describe('Tailor', () => { assert.equal(response.body, '' + '' + - '' + - '' + + '' + + '' + 'hello multiple' + - '' + - '' + + '' + + '' + '' + '' ); @@ -920,11 +896,11 @@ describe('Tailor', () => { assert.equal(response.body, '' + '' + - '' + - '' + + '' + + '' + 'hello many' + - '' + - '' + + '' + + '' + '' + '' + @@ -932,11 +908,11 @@ describe('Tailor', () => { '' + '' + - '' + - '' + + '' + + '' + 'hello exactly three' + - '' + - '' + + '' + + '' + '' + '' + @@ -944,11 +920,11 @@ describe('Tailor', () => { '' + '' + - '' + - '' + + '' + + '' + 'hello exactly three async' + - '' + - '' + + '' + + '' + '' + ''