From d56d9daed983c1358a4cdd571e51f22ff0d8d0e6 Mon Sep 17 00:00:00 2001 From: David Greenspan Date: Fri, 11 Jul 2014 08:28:24 -0700 Subject: [PATCH] Fix view.lookup to not take dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The template compiler assumes that single-segment paths like {{foo}}, compiled into view.lookup(“foo”), don’t take dependencies, returning a function if there is anything reactive going on. We violated that in the Blaze refactor, meaning that #with might re-render its contents when its argument changes! Tests to add: * UI.dynamic doesn’t re-create template when enclosing data context changes. * UI.dynamic doesn’t re-create template when “data” argument changes * Coverage for the “..” and “foo” cases in lookup.js Tests to fix: “simple helper” --- packages/blaze/lookup.js | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/packages/blaze/lookup.js b/packages/blaze/lookup.js index 86a1e4fed4..e7ff5e15ee 100644 --- a/packages/blaze/lookup.js +++ b/packages/blaze/lookup.js @@ -28,6 +28,10 @@ var wrapHelper = function (f) { // If a function is found, it is bound to the object it // was found on. Returns a function, // non-function value, or null. +// +// NOTE: This function must not establish any reactive +// dependencies. If there is any reactivity in the +// value, lookup should return a function. Blaze.View.prototype.lookup = function (name, _options) { var template = this.template; var lookupTemplate = _options && _options.template; @@ -38,7 +42,7 @@ Blaze.View.prototype.lookup = function (name, _options) { if (!/^(\.)+$/.test(name)) throw new Error("id starting with dot must be a series of dots"); - return Blaze._parentData(name.length - 1); + return Blaze._parentData(name.length - 1, true /*_functionWrapped*/); } else if (template && (name in template)) { return wrapHelper(bindToCurrentDataIfIsFunction(template[name])); @@ -47,29 +51,37 @@ Blaze.View.prototype.lookup = function (name, _options) { } else if (UI._globalHelpers[name]) { return wrapHelper(bindToCurrentDataIfIsFunction(UI._globalHelpers[name])); } else { - var data = Blaze.getCurrentData(); - if (data) - return wrapHelper(bindIfIsFunction(data[name], data)); + return function () { + var data = Blaze.getCurrentData(); + if (lookupTemplate && ! (data && data[name])) + throw new Error("No such template: " + name); + if (! data) + return null; + var x = data[name]; + if (typeof x !== 'function') + return x; + return x.apply(data, arguments); + }; } return null; }; // Implement Spacebars' {{../..}}. // @param height {Number} The number of '..'s -Blaze._parentData = function (height) { +Blaze._parentData = function (height, _functionWrapped) { var theWith = Blaze.getCurrentView('with'); for (var i = 0; (i < height) && theWith; i++) { theWith = Blaze.getParentView(theWith, 'with'); } - return (theWith ? theWith.dataVar.get() : null); + if (! theWith) + return null; + if (_functionWrapped) + return function () { return theWith.dataVar.get(); }; + return theWith.dataVar.get(); }; Blaze.View.prototype.lookupTemplate = function (name) { - var result = this.lookup(name, {template:true}); - - if (! result) - throw new Error("No such template: " + name); - return result; + return this.lookup(name, {template:true}); };