Fix afterRender not firing issue with moving from empty template name…

… to a valid template
rniemeyer · Nov 23, 2014 · 2cd71fc · 2cd71fc
1 parent 7032e20
commit 2cd71fc
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 7 deletions.
diff --git a/bower.json b/bower.json
@@ -1,6 +1,6 @@
 {
     "name": "knockout-amd-helpers",
-    "version": "0.7.2",
+    "version": "0.7.3",
     "main": "build/knockout-amd-helpers.min.js",
     "ignore": [
         "examples",

diff --git a/build/knockout-amd-helpers.js b/build/knockout-amd-helpers.js
@@ -1,4 +1,4 @@
-// knockout-amd-helpers 0.7.2 | (c) 2014 Ryan Niemeyer |  http://www.opensource.org/licenses/mit-license
+// knockout-amd-helpers 0.7.3 | (c) 2014 Ryan Niemeyer |  http://www.opensource.org/licenses/mit-license
 define(["knockout"], function(ko) {
 
 //helper functions to support the binding and template engine (whole lib is wrapped in an IIFE)
@@ -207,7 +207,7 @@ if (ko.virtualElements) {
     engine.makeTemplateSource = function(template, doc) {
         var el;
 
-        //if a name is specified, then use the
+        //if a name is specified
         if (typeof template === "string") {
             //if there is an element with this id and it is a script tag, then use it
             el = (doc || document).getElementById(template);
@@ -236,6 +236,11 @@ if (ko.virtualElements) {
             existingAfterRender = options && options.afterRender,
             localTemplate = options && options.templateProperty && bindingContext.$module && bindingContext.$module[options.templateProperty];
 
+        //restore the original afterRender, if necessary
+        if (existingAfterRender) {
+            existingAfterRender = options.afterRender = options.afterRender.original || options.afterRender;
+        }
+
         //if a module is being loaded, and that module has the template property (of type `string` or `function`) - use that as the source of the template.
         if (localTemplate && (typeof localTemplate === "function" || typeof localTemplate === "string")) {
             templateSource = {
@@ -255,6 +260,9 @@ if (ko.virtualElements) {
                     existingAfterRender.apply(this, arguments);
                 }
             };
+
+            //keep track of the original, so we don't double-wrap the function when template name changes
+            options.afterRender.original = existingAfterRender;
         }
 
         return engine.renderTemplateSource(templateSource, bindingContext, options);

diff --git a/build/knockout-amd-helpers.min.js b/build/knockout-amd-helpers.min.js
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "knockout-amd-helpers",
-  "version": "0.7.2",
+  "version": "0.7.3",
   "devDependencies": {
     "grunt": "~0.4.1",
     "grunt-contrib-uglify": "~0.2.0",

diff --git a/spec/amdTemplateEngine.spec.js b/spec/amdTemplateEngine.spec.js
@@ -86,6 +86,25 @@ define(["knockout", "knockout-amd-helpers"], function(ko) {
             });
         });
 
+        it("should call the afterRender function, even if the initial template was empty", function(done) {
+            var vm = {
+                templateName: ko.observable(""),
+                first: ko.observable("Jon"),
+                last: ko.observable("Black"),
+                rendered: jasmine.createSpy()
+            };
+
+            applyBindings("template: { name: templateName, afterRender: rendered }", vm, null, function() {
+                vm.templateName("fresh");
+
+                setTimeout(function() {
+                    expect(container.innerText).toEqual("fresh: Jon Black");
+                    expect(vm.rendered.calls.count()).toEqual(1);
+                    done();
+                }, 50);
+            });
+        });
+
         it("should still first use a template in a script tag", function(done) {
             var template = document.createElement("script");
             template.type = "text/html";

diff --git a/src/amdTemplateEngine.js b/src/amdTemplateEngine.js
@@ -42,7 +42,7 @@
     engine.makeTemplateSource = function(template, doc) {
         var el;
 
-        //if a name is specified, then use the
+        //if a name is specified
         if (typeof template === "string") {
             //if there is an element with this id and it is a script tag, then use it
             el = (doc || document).getElementById(template);
@@ -71,6 +71,11 @@
             existingAfterRender = options && options.afterRender,
             localTemplate = options && options.templateProperty && bindingContext.$module && bindingContext.$module[options.templateProperty];
 
+        //restore the original afterRender, if necessary
+        if (existingAfterRender) {
+            existingAfterRender = options.afterRender = options.afterRender.original || options.afterRender;
+        }
+
         //if a module is being loaded, and that module has the template property (of type `string` or `function`) - use that as the source of the template.
         if (localTemplate && (typeof localTemplate === "function" || typeof localTemplate === "string")) {
             templateSource = {
@@ -90,6 +95,9 @@
                     existingAfterRender.apply(this, arguments);
                 }
             };
+
+            //keep track of the original, so we don't double-wrap the function when template name changes
+            options.afterRender.original = existingAfterRender;
         }
 
         return engine.renderTemplateSource(templateSource, bindingContext, options);