| Issue 22: | JavaScript JSONTemplate cannot be prototyped | |
| 1 person starred this issue and may be notified of changes. | Back to list |
The Template class in JSONTemplate cannot be prototyped.
ie: You cannot add a function to jsontemplate.Template.prototype and make
use of it.
I tried making use of json-template inside of a Server Side environment and
wanted to prototype in a function to save a rendered template to the disk.
Because of the way Template returns a new object rather than acting like a
proper JavaScript class.
Rather than returning a new object Template should do something like this
at the very least:
{{{
function Template(template_str, options) {
if(!( this instanceof Template )) return new Template(template_str, options);
// options.undefined_str can either be a string or undefined
options = options || {};
var program = _Compile(template_str, options);
this.render = function(data_dict, callback) {
var context = _ScopedContext(data_dict, options.undefined_str);
_Execute(program.Statements(), context, callback);
};
this.expand = function(data_dict) {
var tokens = [];
this.render(data_dict, function(x) { tokens.push(x); });
return tokens.join('');
};
};
}}}
Though the proper thing to do would be to make at least expand part of
Template's prototype. Even better if render is part of it and the
constructor is reduced to adding something like `this._program =
_Compile(template_str, options);`.
May 20, 2009
Project Member
#1
gtempacc...@yahoo.com
May 27, 2009
Ok, sorry, I haven't had time to tweak the json-template code till now. This patch should fix the issue. My preference is to make expand and render part of the prototype and set variables like this._options and this._program so that they can use it. Using a closure is still an option if you're really desperate for it. The difference is extreme privacy vs. wasted memory. The closure does hide the program variable and options from being modified from the outside. But I don't see to much purpose for that, there are thousands of other ways you could break anything in JavaScript, the _ prefix is enough to say "Don't touch this, if you do, whatever breaks is your fault." The problem of course with the closure is that for each template completely new functions are created. This wastes memory on extra functions when they could just be common functions to all Template instances. It also means that you can't use the proxy-method pattern to enhance the abilities of the existing methods. And if you don't set this._options then any methods users prototype onto the Template don't have the ability to look at the options used to create the template and act properly based on them.
May 27, 2009
OK, this looks good basically, but are these two lines strictly necessary?
+ if(!(this instanceof Template)) // run as `Template()` instead of `new Template()`
+ return new Template(template_str, options);
You're supposed make a template like:
var t = Template("foo");
not
var t = new Template("foo");
Are you only protecting against the latter case? If so I would just leave it out and
document it well in the examples and tests.
I pretty much learned JavaScript from Crockford's latest book, and he says he never
uses "new" anymore, if I recall correctly.
May 28, 2009
Yes they are necessary. To make use of prototyping new needs to be made use of. Template() itself does not create an instance, so we check and if `this` is not an instance of what we are trying to create we pass it onto an actual construction of the object using new. This effectively means that just like Error() both Template() and new Template() do precisely the same thing. It's especially necessary for Template() to work, otherwise only `new Template()` can be used.
May 31, 2009
So, I went to apply this patch, but I need something to write in the unit test to verify it. This got me down this rathole of trying to understand all of JavaScript's idioms for inheritance, e.g. http://javascript.crockford.com/prototypal.html and http://brehaut.net/miscellany/Development/Javascript/Crockford_vs_Base2, etc. I think what you're initially suggesting is just to do: jsontemplate.Template.prototype.writeToFile = function (filename) { ... } or jsontemplate.Template.prototype.expand = function (filename) { ... } I'm not sure I want to recommend this to people because it's a global mutation that can break other modules using jsontemplate. It's not a big deal for smaller programs, but the implementation is also meant to be used with server side JS, and thus larger programs. So what inheritance idiom do you have in mind for people who don't want to directly modify jsontemplate.Template.protoype? Can you provide this code with the patch? Crockford's solution seems to involve his special Object.create method, and it seems like a bad idea to foist this on people.
May 31, 2009
I'm just saying to support prototyping the template like any other JavaScript class.
It's up to people whether or not they want to prototype or just create local
functions of their own.
For those wanting to inherit, something like this should theoretically work:
var JSONTemplate = (function(jsontemplate) {
function JSONTemplate(template_str, options) {
if(!(this instanceof jsontemplate.Template))
return new JSONTemplate(template_str, options);
jsontemplate.Template.call(this, template_str, options);
}
JSONTemplate.prototype = new jsontemplate.Template;
return JSONTemplate;
})(jsontemplate);
The wrapping function just makes sure that nothing breaks if someone decides to
change the global jsontemplate variable to something else.
It's just the common trick of using a call/apply on a constructor passing this to it,
plus the same bit I noted for the actual jsontemplate to make the function work
without the new keyword.
May 31, 2009
What I was getting is that there should be a standard idiom for overriding methods without mutating globals (this doesn't seem to be very standardized in JavaScript). I haven't seen this pattern you're showing there; it seems a bit complicated for general use. But I've just made a TODO in the code for later, and applied the patch. The test in browser_tests.py specifies what I think you want -- please verify it because we could change the structure again, and I'll only go off that test to avoid breaking it. Thanks for the patch.
May 31, 2009
Ok the test is alright. Though just for sanity's sake you might want to test it with
both new ... and without the new, to make sure that use with and without the new is
consistent. After all the current thing to do is use it without the new.
For a pattern for overriding methods without mutating globals, yes it is unfortunate
but that isn't something that really exists as a standard in JavaScript. Normally
either you MonkeyPatch the global, or you just create your own local function.
If you want, you can make that idiom easier to do with something like this in the
json-template.js file:
Template.newTemplateClass = function() {
var Template = this;
function JSONTemplate(template_str, options) {
if(!(this instanceof Template))
return new JSONTemplate(template_str, options);
Template.call(this, template_str, options);
}
JSONTemplate.prototype = new Template;
JSONTemplate.newTemplateClass = this.newTemplateClass;
return JSONTemplate;
};
Then someone would just do:
var MyTemplate = jsontemplate.Template.newTemplateClass();
MyTemplate.prototype.myOwnMethod = function() { ... };
var tpl = MyTemplate('...');
tpl.myOwnMethod();
May 31, 2009
Oh right... Just so you know it's fine to just use my real name in commit comments "Daniel Friesen". It's easier to refer to me by name, "Nadir Seen Fire" is my newer more unique nick, but a lot of people still refer to me by dantman.
May 31, 2009
Actually the 50+ existing tests all instantiate it without 'new', so I just added a case with 'new'. I'll probably just wait for some more feedback about inheritance. It's not really specific to the template problem, so now that we're doing the standard thing, we can just leave it up to the user to pick their pattern.
Status:
Fixed
Jun 1, 2009
Yup, I was just mentioning it for sake of the prototype test.
ie: in the unlikely edge case that:
Template.prototype.myFunc = function() { return this.expand({my:'output'}); };
new Template('{my}').expand(); // works
Template('{my}').expand(); // fails
As a result of code changing for some reason making creating a template work but not
actually be an instance and inherit the prototype.
I guess it's just me and seeing to many of the slim possibilities.
|