Helpers refactoring w/ stateful buffers, soon to contain live binding hookup

[rjgotten]

An initial version of our refactoring to support live binding, as disussed in #8.

Stateful rendering context using Helpers calss

We've t turned the static vash.helpers into the vash.Helpers class. This passes all current unit tests, does away with the live/die hack and internalizes buffer instances to the Helpers instances.

The latter effectively creates rendering contexts with separate state. Check out the changes to the helpers.layout.js file as well; we've solved the composition problem simply by using the nested compiled templates as actual nested templates (which now have proper nested, separate buffers).

The actual vash.Helpers class constructor is currently internal. Only the prototype is exposed, so users can add additional members to it. We did this to prevent users from mucking about with the internal state of the template engine (Ofcourse, Foo.prototype.constructor == Foo so you can never really make it internal ...)

Live binding hookup

This is coming; still working on it.

Known issues

For the moment, the above changes break the capability to have pre-compiled templates. (There is no way to pass in a valid Helpers instance as the second parameter to a template, and since the helpers now carry the isolated buffer, you really must have them and have them constructed in a controlled manner.)

Probably it will be necessary to expose three functions if you want pre-compiled templates:

• vash.compile -- to compile the function
• vash.link -- to link the compiled function with a helpers instance (see `linkedFunc in the compiler)
• vash.build-- compiles the function, links it with the helpers and returns it ready for use.

This way, you can just do vash.compile( <template> ).toString() to get the pre-compiled function. Before it is used, you have to run vash.link( <func> ) to get the linked function (which you can run).
vash.build would be a shorthand that, when offered a string, would run compile before link and would just run link otherwise.

Ofcourse, we could also just roll this into the existing vash.compile function, but it feels better to use the actual correct terms. (compile + link == build)
What is your opinion on this?

A note on the compiler rewrite to use regex token replacements

It's a nice idea to increase readability, however you may want to reconsider the actual token strings. I'd atleast make them a bit more robust by adding a control character you are unlikely to have have inside regular template body text. (Honestly, it feels like you're sacrificing correctness / stability for speed though. While Vash's origins are Razor, you may not want it to live that close to the edge...)

[kirbysayshi]

I believe I specifically added this line relatively recently. Is there a reason for its removal?

[kirbysayshi]

The reason I added it was to correctly format error reporting for line numbers less than 10 (e.g. make them line up)

[kirbysayshi]

Awesome, I think this is on a great path. Once I hear back on the comment in vruntime.js, I'll merge.

Regarding vash.Helpers being internal

Is this necessary? I understand the attempt at preventing mucking about, but given that there is still a direct route in via vash.helpers.constructor, I wonder if it's better to be obvious and expose it.

Regarding compile + link

I think the primary goal should be "least surprise". vash.compile( str, options ) was originally chosen because that's what express >= 2 < 3 would look for in a "compatible" template engine. I think prior to that it was vash.template. "Build" vs "compile" are confusing, because in colloquial usage they're used interchangeably. So I'm open to changing the API, but for each step I want the purpose to be clear based on the name. VCompiler#assemble is named that way because I thought it was confusing to have two .compile functions. :)

What about:

• VCompiler#generate returns the string function
• VCompiler#assemble links in helpers, overwrites toString (and aliased as vash.link)
• vash.compile performs the previous steps

Regardless, if we can always assume closures are valid, then I think your vash.link is on the right path, especially because it allows for a non-global vash:

1. vash.link( fn ) where fn( model, HelpersConstructor )
2. vash.link has an alternate constructor accepting an array of fn (possibly?)
3. tpl.toClientString returns the string vash.link( function(model, Helpers){ ... } ) (possibly?)

The vash cli helper would be modified to either wrap each tpl.toString() in vash.link or a single call if the array version proves to be faster. JSPerf results practically even out, where X is faster in FF, Y is faster in Chrome.

I did wonder if it would ever be beneficial to pass in an instance of Helpers. I can't think of a use case right now, but I suppose that could be solved with an alternative linkedFunc signature, something like ( model, helperInst ) if needed.

Regarding compiler rewrite note

I actually didn't do it for speed, but primarily to make it easier to understand what was going on as a developer. Dealing switching back and forth between quotes, static strings vs dynamic properties, etc. was just getting really messy. In previous jsperf tests (there's a link in the pre-rewrite code) __vo.push was faster than __vout +=, unless you were able to do __vo = x + y + z (never reassigning, just concatenating).

I agree though that the names are potentially dangerous without control characters. Again, I was trying to keep it readable and easy to type, as the templates got complicated with helper additions. Alternatives could be %VHNAME%, >VHNAME, #vhname, others?

Which part feels like I'm sacrificing correctness?

I think we need to break out these discussions into more than one issue 8)

[rjgotten]

Regarding vash.Helpers being internal

The vash.Helpers constructor was made internal to -- let's say; discourage, people from trying to manually construct a Helpers instance. Also, unless you're dealing with a full-on class based library, hiding the prototypal parts of extending the library is a good idea, since not everyone is comfortable with writing .prototype.foo for extension. So, it's more of a KISS and developer guidance thing.

Regarding compile + link
Keeping the API as consistent with the current as possible is ofcourse good. Your suggestion fits the bill nicely.

Regarding compiler rewrite note

Which part feels like I'm sacrificing correctness?

I meant the 'hackiness' of using string substitution for placeholder tokens, which may cause problems if you have a token format that is too general. You have to consider that users may use string formatting functions inside the template (formatting could be a presentation/view concern and not a model concern) and thus all the general formatting masks such as %FOO%, #FOO, {FOO} etc. are all susceptible to being mangled.

Imho you should probably be looking for something more exotic that a user is never expected to enter as a regular part of a template. (E.g. stuff like the zero width joiner character.)

[kirbysayshi]

Alright cool, vash.Helpers will be internal.

Regarding compiler strings

I hadn't considered string formatting functions and the possibility for them to either mangle or be mangled. In that case, I wonder if just a straight string actually makes more sense, similar to what's used now: VVHELPNAMEVV, VVMODNAMEVV, VVTPLBODYVV.

Do you have an example of how something could mangle or destroy a template? I could see this being bad (imagine documentation):

And the string <a href="">VVMODNAMEVV</a> is used to ease code readability

So I guess I'm looking for an example that is not Vash documentation.

I'd want to avoid something super exotic, because typing a bunch of unicode escape sequences isn't any easier to do or read than before :) but it's an interesting idea... maybe something like /u2622MODELNAME/u2622 (☢ U+2622)?

Or I suppose I could just use an array again.

[kirbysayshi]

I'll fix the reportError code myself just to expedite the other planned additions. We can keep discussing compiler stuff here for now, but live binding should be a separate pull request if it requires vash changes.

[rjgotten]

live binding should be a separate pull request if it requires vash changes.

I was already planning on having it as separate delivered pull requests. Hence the early commit of the stateful vash.Helpers when I was finished with it.

The reportError problem was a fluke on my end: something was lost while I was working on merging the two code bases.

Regarding the string substitution: you could always go back to using string concat for the template body code and only use the replacement technique with the head/foot boilerplate before inserting the template body. That would avert any potential problems with user-inserted comment matching a replacer token. The boilerplate was the biggest eye-sore with the concatenation anyway.

[kirbysayshi]

Ha, I had basically the exact same idea! I'll get to work on it.