mustache / spec

The Mustache spec.
MIT License
368 stars 71 forks source link

"Interpolation - Multiple Calls" test should be implementation-independent #30

Closed cjerdonek closed 11 months ago

cjerdonek commented 12 years ago

I ran into this issue in the process of refactoring. The "Interpolation - Multiple Calls" test is currently written in a way that assumes tags are evaluated left to right:

template: '{{lambda}} == {{{lambda}}} == {{lambda}}'
expected: '1 == 2 == 3'

The test should probably be rewritten to be implementation-independent (unless there is a left-right requirement that I'm not aware of). If there is a left-right requirement, it is still probably better to rewrite so that the test is more specific and not testing two unrelated things.

pvande commented 12 years ago

Hmm... Now that's a fascinating problem. Let's try to decompose it a little.

We've already stated that lambdas should be re-evaluated on each call. If we assume that a strict left-right order is not guaranteed, then the output of multiple calls becomes difficult to predict (and potentially non-deterministic). Since left-to-right evaluation matches user expectations best, that should be preferred first.

If we do have a strict left-to-right evaluation, then, as you point out, there are multiple assertions to be made about behavior: tags are interpolated left-to-right (which only ends up being visible when calling the same lambda more than once), and lambdas should always be re-evaluated. Testing either of those behaviors implicitly depends on the behavior of the other -- you cannot assert the output of uncached lambdas unless you know the order, and you cannot test the order of multiple lambda calls unless those calls have different values (and so are uncached).

Additional thoughts:

Out of curiosity, what optimizations were you finding around alternate evaluation orders?

cjerdonek commented 12 years ago

I'll answer your other questions a bit later. But let's start with the question of "what might an implementation-independent test look like?" I think there are a number of possibilities. It sort of depends on what's easiest and what style you're after.

One possibility is to wrap the lambda in a list to force an ordering:

template = '{{#list}}{{lambda}}{{/list}}' 

Another possibility is to construct a lambda that, say, returns the number of calls only if the number of calls is at least three, and returns an empty string otherwise. Then the expected value of the following--

template = '{{lambda}}{{lambda}}{{lambda}}' 

would be--

'3'

This all raises another question though that I think isn't covered explicitly by the spec. Are implementations prohibited from calling lambdas more than the minimum number of times? What if someone wrote an implementation that called lambda twice in some instances -- perhaps to do a value check prior to some optimization or something? My instinct is that that should be allowed, in which case the tests should also be independent of that possibility (the ones I gave above are not).

cjerdonek commented 12 years ago

Regarding left-right evaluation, it is possible to test left-right evaluation independent of testing non-caching (though my sense is that evaluation order should not be part of the spec). Here is one way (in Python) -- modifying the example in the spec:

set_first = lambda label: globals().get("first") or globals().update(first=label) or ""

template = '{{lambda1}}{{lambda2}}'
context = {'lambda1': set_first(1), 'lambda2': set_first(2)}
sayrer commented 12 years ago

left-right seems pretty obvious. sort of like top-bottom.

{{lambda}} {{{lambda}}} {{lambda}}

cjerdonek commented 12 years ago

If the spec is going to weigh in on evaluation order, I think it would likely be opening a can of worms. Templates are actually trees. So would that mean the spec would also need to specify depth-first versus breadth-first traversal? And would the left-right requirement apply across all tag-types, or just within tags of the same type? How about lambdas inside partials? The spec would have to be clear on every case. But by that time, the spec would probably be dictating the parsing algorithm itself.

sayrer commented 12 years ago

Since sections can map to sequences in the context, can also be nested, most of those questions are already answered.

groue commented 12 years ago

Some mustache implementation (one of the java ones, if I remember well) renders templates in a concurrent fashion. This is an interesting way to make non-obvious the left to right ordering. However cjerdonek is right about the fact that the spec should state something like "the rendered string should be as if all mustache tags were rendered in a left-to-right depth-first tree traversal" (leaving implementors add optimizations options like : "if your lambdas are purely functional, you may set the concurrent flag to true, and take profit of your multicore CPU".

groue commented 12 years ago

The concurrent mustache java implementation is https://github.com/spullara/mustache.java by @spullara.

cjerdonek commented 12 years ago

For background purposes, what are some of the real-world use cases for not caching lambda return values (i.e evaluating every time)? That would help inform any statement in the spec re: ordering.

Re: @groue --

the spec should state something like "the rendered string should be as if all mustache tags were rendered in a left-to-right depth-first tree traversal"

Thanks for offering this. For completeness, any statement like this should probably also include something like "and as if all list-valued section tags were rendered sequentially."

Re: @pvande --

Out of curiosity, what optimizations were you finding around alternate evaluation orders?

It wasn't an optimization. It was more just a byproduct of the order in which I approached some refactoring. I began with the code path for literals, which caused literals (non-escaped values) to render before non-literals (escaped values). This caused the middle {{{lambda}}} in the spec test case to evaluate first.

groue commented 12 years ago

@cjerdonek Nice addition :-) I like it when a spec says "as if", since it means that the specifiers has well understood that implementors need room, and users assurance.

spullara commented 12 years ago

Thanks for alerting me to this thread @groue.

Concurrent evaluation is certainly one of the theoretical advantages of Mustache over other templating systems due to its lack of logic in the template. Specifying that things are called in a set order is problematic for me. I would rather have the specification take the position that all variables in a scope and even individual list sections are evaluated in a non-deterministic order to be optimized by the implementation. We would be much better served in the long term by making mustache declarative / functional rather than procedural. That said, mustache.java doesn't currently execute value replacement out of order and this test would currently pass. Which reminds me, now that the spec is much more mature, it is probably time that I add it to the test cases for my implementation.

sayrer commented 12 years ago

I think the main issue is that the non-tag source text will usually be ordered, so the evaluation order of the tags does matter. For example:

{{#section}}
<tr>
     <td class=a>{{lambda}}</td>
     <td class=b>{{{lambda}}}</td>
     <td class=c>{{lambda}}</td>
</tr>
{{/section}}

I like the 'as if' language too, fwiw. There are certainly cases where you want to do parallel rendering, but I think lambda tags that depend on shared global state in a parallel implementation are the edge case we should not address ('doctor, it hurts when I do that...')

spullara commented 12 years ago

You can evaluate the tags out of order and output them in order to good effect if they are parallelizable. For example, if each of those lambda's is a network call.

Sam

On Tue, Jan 3, 2012 at 11:53 AM, Rob Sayre < reply@reply.github.com

wrote:

I think the main issue is that the non-tag source text will usually be ordered, so the evaluation order of the tags does matter. For example:

<#section>

{{lambda}} {{{lambda}}} {{lambda}}

I like the 'as if' language too, fwiw. There are certainly cases where you want to parallel rendering, but I think lambda tags that depend on shared global state in a parallel implementation are the edge case we should not address ('doctor, it hurts when I do that...')


Reply to this email directly or view it on GitHub: https://github.com/mustache/spec/issues/30#issuecomment-3345112

sayrer commented 12 years ago

Sure, I think that's where the 'as if' language comes in.

groue commented 12 years ago

@defunkt, thoughts ?

Re: @sayrer

but I think lambda tags that depend on shared global state in a parallel implementation are the edge case we should not address

On the contrary, they should explicitely be adressed by the spec, because both mustache users and implementors should know what happen when such lambdas enter the game. Most tickets in this repo are about unclear behaviors, underspecified edge cases, and cross-implementation compatibility issues. Users deserve clarification, and so do implementors.

pvande commented 12 years ago

As alluded to already, the reason evaluation order matters is that method calls and lambdas are not guaranteed to be purely functional. Furthermore, if all executing code were purely functional, there would no need to worry about the number of times code was executed.

While that may be a reasonable statement for us to make in v2.0.0, side-effects are absolutely permitted in v1.x. I will update the specs to that end soon.

sayrer commented 12 years ago

Sam actually convinced me that specifying evaluation order would be a mistake.