Writer.prototype.parse to cache by tags in addition to template string

[raymond-lam]

Resolve #617
Because the results of Writer.prototype.parse depends on both the template string and the tags, it should cache by both

[dasilvacontin]

LGTM, good catch!

[dasilvacontin]

These are parenthesis, not braces, right?

[raymond-lam]

Yes, my bad. I'll fix it.

[raymond-lam]

@dasilvacontin Addressed your comment -- good catch! I actually meant to test the template with Mustache.tags as the delimiters, with it being braces.

[dasilvacontin]

LGTM, thanks for the fix! :)

[raymond-lam]

@dasilvacontin Awesome, thanks!

[dasilvacontin]

I started refreshing my memory about the other PR, but I don't have enough time today. Will do tomorrow morning.

[raymond-lam]

@dasilvacontin no rush! Thanks!

[petrkoutnysw]

I am afraid this PR caused breaking change for us.
We are doing following:

1. parse our template with custom tags
2. render the template - according to mustache.js:465 our custom tags are not used, hence the tokens are different and custom tags are not recognized

[phillipj]

@petrkoutnysw a bug fix landed after this got merged: #664. This is also what's included in v2.3.1 of mustache.js.

Are you sure you're using the latest version? Or have I misunderstood and you have found another bug?

[petrkoutnysw]

@phillipj Yeah I have 2.3.1.

It is quite easy - if you take a look at parse function (line 465), it creates a cache using tags if cache does not exist already. Then you call render function that calls parse internally without specifying tags. Thus it is getting wrong tokens. Hope it makes sense, I have not been reading mustache code previously.

Anyway, we froze our mustache to 2.3.0.

[raymond-lam]

@petrkoutnysw Do you have replication steps (the template you used and how you made the call to render it)?

[raymond-lam]

@petrkoutnysw

It tried the following code:

mustache.parse('<<foo>>bar', ['<<','>>']);
mustache.render('<<foo>>bar', { foo:'biz'})

Which outputs <<foo>>bar as expected.

Then, if I do:

mustache.tags=['<<','>>']
mustache.render('<<foo>>bar', { foo:'biz'});

It outputs bizbar again as expected.

How are you setting the custom tags (there's only one way as far as I can tell) and calling the render function?

render does indeed call parse without passing in a tags argument, but the cacheKey in parse is defined to fallback to mustache.tags. (As far as I can tell, the tags argument is never used internally by mustache.js.)

445   Writer.prototype.parse = function parse (template, tags) {
446     var cache = this.cache;
447     var cacheKey = template + ':' + (tags || mustache.tags).join(':');
448     var tokens = cache[cacheKey];
449
450     if (tokens == null)
451       tokens = cache[cacheKey] = parseTemplate(template, tags);
452
453     return tokens;
454   };

[petrkoutnysw]

@raymond-lam Give me some time please, I am on conference and don't have time to reply right now. Thanks.

[phillipj]

Maybe these two gives valuable input here? #669 & #662

[raymond-lam]

@phillipj @petrkoutnysw Ah yes I see those reported issues now -- I'll have a look.