search

mustache / spec

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

Add template inheritance spec #75

Closed jazzdan closed 3 years ago

jazzdan commented 10 years ago

This adds a specification for hogan.js style template inheritance. As of time of writing, three mustache implementations are 100% compatible with this implementation:

Here's how it works:

{{! header.mustache }}
<head>
  <title>{{$title}}Default title{{/title}}</title>
</head>
{{! base.mustache }}
<html>
  {{$header}}{{/header}}
  {{$content}}{{/content}}
</html>
{{! mypage.mustache }}
{{<base}}
  {{$header}}
    {{<header}}
      {{$title}}My page title{{/title}}
    {{/header}}
  {{/header}}

  {{$content}}
    <h1>Hello world</h1>
  {{/content}}
{{/base}}

Rendering mypage.mustache would output:

<html><head><title>My page title</title></head><h1>Hello world</h1></html>

A full specification can be seen below.

At @Etsy, much like @Twitter (see #38), we often want to have one template where we maintain the "bones" of the page and each real page on the site merely replaces sections of that page with appropriate content. A simple example of this can be seen above, where a page can grab base, getting all of our analytics scripts and other markup that every page needs, while retaining the ability to swap in a completely different header. Alternatives, such as copying the bones to every top level template, or simulating this behavior with a multi-step mustache render on the server, are not ideal.

Etsy is in the process of rolling out the new version of mustache.php that supports template inheritance to our production cluster after testing this implementation extensively. Hogan.js and mustache.java are both being used in production at Twitter.

Many thanks to @dtb, @mdg, @bobthecow for their assistance. Special thanks to @spullara for quickly making mustache.java compatible with this spec.

bobthecow commented 10 years ago

Hey, @groue — I remember you saying GRMustache was moving to this style of inheritance as well... Do you mind running this spec against it and seeing how it works?

groue commented 10 years ago

Hi @bobthecow. Yes, GRMustache passes all those tests (modulo white-space conformance, that GRMustache has never focused on).

Generally speaking, GRMustache passes all inheritance tests from this PR, from hogan.js, and spullara/mustache.java.

I'm curious about your own implementations' conformance to GRMustache tests as well - they cover a few cases not included in this PR:

groue commented 10 years ago

@jazzdan wrote:

As of time of writing, three mustache implementations are 100% compatible with this implementation:

There are four or them, actually :-) You can add groue/GRMustache.

bobthecow commented 10 years ago

Your third bullet (separate namespace for inheritance vs. regular variable keys) was addressed in the PHP implementation.

bobthecow commented 10 years ago

@groue This one is interesting, and I'm not sure I'm convinced it's the right way to treat it:

https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/inheritable_partials.json#L59-L64

Sell me on it :)

bobthecow commented 10 years ago

I just tested the PHP implementation against your second bullet point (several conflicting inheritable sections), and that works as well.

bobthecow commented 10 years ago

Okay. Your inheritable partials tests mostly pass in the PHP implementation (2 failures, 5 errors). The two failures are due to treatment of blocks inside sections (my "not convinced" comment above).

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}
groue commented 10 years ago

Thanks for your time @bobthecow :-)

https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/inheritable_partials.json#L59-L64

Sell me on it :)

Generally speaking, I tend to think that this behavior is required by those two arguments:

The two arguments join on the "context" word: if the layout template changes the context, inherited sections need to render in this context. Hence the test you question, and the example below:

layout.mustache

{{!--
Layout for a document.

Exposed inheritable sections:
- $title: What to be rendered for the document title.
          When rendered, the `document` object is at the top of the context stack.
          Default value: `document.title`.
}}
{{#document}}
    {{$title}}
        {{title}}
    {{/title}}
    ...
{{/document}}

essay.mustache

{{<layout}}
    {{$title}}
        {{! Prepend "Essay" to the document title }}
        Essay: {{title}}
    {{/title}}
{{/layout}}

Rendering:

render('essay', { document: ...});
groue commented 10 years ago

The five errors are exceptions thrown because Mustache.php, like Hogan.js, throws errors for things like this:

{{< parent }}
  {{# section }}this will throw an exception!{{/ section }}
{{/ parent }}

I see. What is the intent? To prevent users to embed ignored content? If so, do you throw exception for plain text as well? If not, what is the intent of you exception?

{{< parent }}
  Not only sections, but this text also is ignored.
{{/ parent }}
bobthecow commented 10 years ago

@groue I'm actually leaning toward that (see comment). The reason it came up was things like this:

{{< parent }}
  {{# items }}
    {{$ block }}wheee!{{/ block }}
  {{/ items }}
{{/ parent }}

… which are weird. I'd be in favor of explicitly whitelisting only a few things that can live in the top level of a parent tag:

groue commented 10 years ago

@bobthecow All right, I understand, and I'm not against returning errors for all ignored tags, but the ones you are listing: white space, comments, inheritable sections, and change-delimiter tags.

BTW, GRMustache also has tests for such parsing errors: https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/tag_parsing_errors.json & https://github.com/groue/GRMustache/blob/GRMustache7/src/tests/Public/v7.0/Suites/groue:GRMustache/GRMustacheSuites/expression_parsing_errors.json

Back on your exceptions:

There are only six GRMustache inheritance tests which involve regular sections:

Only 1 of them embeds a regular section inside a inheritable section, and has your implementation throw an exception. Let's forget about this one:

The others should not raise any exception. I mean, your implementation does not have to handle them, but you may be interested in looking at them more precisely:

bobthecow commented 10 years ago

Those aren't the ones that raised exceptions, these are:

[
  // ...
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{$inheritable}}4{{/inheritable}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "4"
  },
  {
    "name": "The content of the last inheritable sections in partials is rendered in the inherited section",
    "data": { },
    "template": "{{<partial1}}{{$inheritable}}1{{/inheritable}}{{>partial2}}{{/partial1}}",
    "partials": {
      "partial1": "{{$inheritable}}ignored{{/inheritable}}",
      "partial2": "{{$inheritable}}2{{/inheritable}}{{$inheritable}}3{{/inheritable}}" },
    "expected": "3"
  },
  // ...
  {
    "name": "Partials in inheritable partials can override inheritable sections",
    "data": { },
    "template": "{{<partial2}}{{>partial1}}{{/partial2}}",
    "partials": {
        "partial1": "{{$inheritable}}partial1{{/inheritable}}",
        "partial2": "{{$inheritable}}ignored{{/inheritable}}" },
    "expected": "partial1"
  },
  // ...
  {
    "name": "Templates sections can not override inheritable sections in inheritable partial",
    "data": { "section": true },
    "template": "{{<partial}}{{#section}}inherited{{/section}}{{/partial}}",
    "partials": { "partial": "{{$section}}success{{/section}}" },
    "expected": "success"
  },
  // ...
]

Note that they all include tags besides inheritable sections inside parent tags, which is the current criteria for throwing an exception.

I did just realize that two of these revolve around allowing partials to define inheritable sections. I'm inclined to push back against this one and say only inheritable sections actually defined inside the parent tag count.

groue commented 10 years ago

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

bobthecow commented 10 years ago

All right, you can do as you wish.

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

bobthecow commented 10 years ago

All right, you can do as you wish. My take on that point is as stated above:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial. Why? Because one should be able to extract a partial without any worry, as long as the resulting templates are syntactically correct.

But that's currently not the case. Consider this template:

{{=<% %>=}}
<% win %>
{{ fail }}

You can't extract any one line of this template into a partial, because the whole thing will break.

I'm just saying that I feel like inheritable sections are a special case too.

groue commented 10 years ago

That's not constructive. I'm trying to have a conversation and come to a consensus about common functionality so that we can actually standardize this behavior.

I'm sorry. I did not want to pollute @jazzdan's PR. I did state my reason after. Your answer:

But that's currently not the case.

Yes, you are right. Set delimiter tags ruin everything. I wish they were not there. I never use them. They are a hack. That's why I think my take is still valid. And this is why GRMustache tests for snippet-extraction-into-partials at several levels. So that I'm sure my users can extract whatever they want into a partial, as long as it looks sensible to them. Who am I to say it's forbidden? Not implemented, why not. But forbidden? When they have a legitimate need? When it can be implemented in a rigourous way? When it has been implemented?

You may not include those tests in this spec. This is what I meant by "you can do as you wish". But I won't remove this feature from GRMustache.

groue commented 10 years ago

About free partials extraction, let me quote @defunkt on http://mustache.github.io/mustache.5.html

In this way you may want to think of partials as includes, or template expansion, even though it's not literally true.

For example, this template and partial:

base.mustache:
<h2>Names</h2>
{{#names}}
  {{> user}}
{{/names}}

user.mustache:
<strong>{{name}}</strong>

Can be thought of as a single, expanded template:

<h2>Names</h2>
{{#names}}
  <strong>{{name}}</strong>
{{/names}}

This is the spirit.

bobthecow commented 10 years ago

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

groue commented 10 years ago

Allowing partials inside parent tags changes what would be a parse error ("unexpected section tag inside a parent tag!") into a runtime exception, because you can't be sure that a partial won't include something that isn't allowed directly inside a parent tag. It complicates things :)

What runtime exception are you talking about? I'm not aware of runtime errors I should raise when rendering plain Mustache templates.

In GRMustache, the only runtime errors are for missing filters (a feature added by GRMustache, not in the spec: unknown filter f in {{ f(x) }}), and inconsistent mixing of HTML and text, a pretty rare programming error that can happen in GRMustache only (text templates are not in the spec: https://github.com/groue/GRMustache/blob/master/Guides/html_vs_text.md).

bobthecow commented 10 years ago

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
groue commented 10 years ago

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

bobthecow commented 10 years ago

This thing you just said:

Well, in your last example, we would have a parse error because the {{> partial }} tag would be disallowed inside the {{< parent }}...{{/}} tag. So we still have no runtime error here.

Directly contradicts that other thing you keep saying:

if a given string is a valid template, then any substring that leaves open/close tags intact can be extracted to a partial, or an inheritable partial.

… and, in fact, agrees completely with what I'm trying to say. So maybe we misunderstand each other :)

groue commented 10 years ago

@bobthecow, I think you mix two questions:

The two topics are different.

Your last example talks about the white-list. Since partial tags should raise a parsing exception, according to this white-list rule, this example would not raise any runtime exception.

About the second topic: please consider that all the partial-extraction tests are rendered by GRMustache. They work. They have a meaning. They test the fact that one can extract a partial from any template snippet, as long as the template snippet can be rendered, and that the two new templates are valid.

bobthecow commented 10 years ago

I was, and have been, talking about the four tests with exceptions I cited above. They throw exceptions because they include tags that are not on the whitelist. I thought you were talking about them as well, and saying that they should be valid.

groue commented 10 years ago

The three first tests are about choosing which overriding section wins, when several ones could be rendered. They make sure the last one wins. They do not involve the white-list.

The last test could raise an exception, with the white-list. Today, it just makes sure an regular section is ignored, and does not mess with inheritance.

bobthecow commented 10 years ago

… partial tags should raise a parsing exception, according to this white-list rule…

All three of these raise exceptions, because they all have partials tags directly inside parent tags:

{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
  {{$inheritable}}4{{/inheritable}}
{{/partial1}}
{{<partial1}}
  {{$inheritable}}1{{/inheritable}}
  {{>partial2}}
{{/partial1}}
{{<partial2}}
  {{>partial1}}
{{/partial2}}
groue commented 10 years ago

OMG let me check my tests

groue commented 10 years ago

Ha yeah, I got it :-)

Those tests are a consequence of general partial extraction. Given the following template:

template.mustache
{{< parent }}
    {{$ inheritable}}...{{/ inheritable }}
{{/ parent }}

The user may want to extract the content into a partial:

template.mustache
{{< parent }}
    {{> partial }}
{{/ parent }}

partial.mustache
{{$ inheritable}}...{{/ inheritable }}

So if I come back to the white-list, I would state the partial tags should be white-listed as well.

groue commented 10 years ago

The last example may look contrived. I have to imagine somebody who wants to reuse common inheritable sections for various layouts, and then extracts them into a partial.

bobthecow commented 10 years ago

Allowing partial tags in parent tags changes what would be a parse error ("unexpected section tag inside parent tag!") into a runtime error ("unexpected section tag inside partial tag included inside parent tag!").

If things like section tags are not allowed directly inside parent tags, this would be a parse error:

{{< parent }}
  {{# foo }}...{{/ foo }}
{{/ parent }}

But this would be a runtime error:

{{< parent }}
  {{> partial }}
{{/ parent }}

partial.mustache

{{# foo }}...{{/ foo }}
groue commented 10 years ago

Your implementation may have limits, that GRMustache has not. Again, do as you want. I'm not the non-constructive guy, this time.

groue commented 10 years ago

As a temporary conclusion: we have four implementations which share a lot of common behaviors. It looks that GRMustache differs from mustache.php and hogan.js in a few aspects:

Those feature proposals and tests are available for anybody in the community who would be interested in them.

wheresrhys commented 10 years ago

What's the status on this. Are there still ongoing discussions offline? Has it completely stalled?

jazzdan commented 10 years ago

Thanks for all the feedback everyone!

My goal with proposing this spec change wasn't to add new features to mustache. Instead, I was aiming to canonicalize a feature that already exists in four major implementations.

Adding a spec, no matter how minimal, for a feature that four out of the top ten popular mustache implementations support would be a good first step to a more healthy mustache ecosystem.

What's the best path forward to getting this merged?

bobthecow commented 10 years ago

I don't know that it would get merged into the actual spec until v2, whenever that happens, but I think adding a proposed folder where we can document (and at least partially canonicalize) proposed specs for things like template inheritance, filters, and anchored dot notation makes a lot of sense sense. What do you think @pvande @locks @groue @janl?

thedeeno commented 10 years ago

I don't know that it would get merged into the actual spec until v2, whenever that happens

Isn't the above proposal an enhacement? What part breaks the existing public api? Seems like this could be merged sooner.

.

Lack of template inheritence is primary drag on mustache adoption for me. I'd love to see this in the spec; and soon.

I feel like there's too much "what if" anlaysis going on in this (and other) threads. Clearly this is a useful feature that's being used in production and is actually supported by a few major implementations in a compatible way. Failure to merge the common parts of these feature into the spec makes me nervous that the mustache ecosystem is fragmenting - making the case for mustache weaker.

People may not actually get bitten by the edge cases above. Adding what's already here to the spec will help us see wider adoption (and likely improvement) of this feature over time.

Thanks @jazzdan for kick starting this and everyone else who's been deliberating here for trying to make mustache more awesome. That's just my 2cents.

bobthecow commented 10 years ago

It's not actually compatible, it restricts previously valid tag names and actually conflicts with some implementations (e.g. R) which rely on the currently acceptable tag names. It also redefines an existing Mustache tag ({{< foo }}) to mean "parent" instead of being a synonym for {{> foo }}.

thedeeno commented 10 years ago

@bobthecow Aha, I see. That wasn't obvious to me after reading a few thread on this. The < breaking change DOES seem unnecessary - especially since it's easy to just pick some other delimiter. Easy to fix for the spec though right?

I'm not sure I understand the 'restricting currently acceptable tag names' case. Wouldn't you only run into these restrictions if you're leveraging inheritence (meaning old templates won't break)?

bobthecow commented 10 years ago

If you have a template with an interpolation tag {{$foo}} (which is allowed by the spec) it'll be treated as a block by a Mustache implementation which supports template inheritance. The same thing will happen no matter what the sigil you choose for any new tag. Because everything that's not explicitly used for something else is allowed by the spec, adding any new tag types to Mustache will break backwards compatibility.

nadako commented 8 years ago

I just added support for this in hxmustache. It passes all spec tests.

Danappelxx commented 8 years ago

My Swift implementation, MuttonChop, also just implemented template inheritance (conforming to this spec). Maybe its time to push for Mustache v2 and make this official?

bubach commented 5 years ago

Wow, I really dislike this concept. The "mypage.mustache" file from the OP is basically a layout configuration expressed as a template - why would you ever want to do this? Views can be instantiated and nested in exactly the same way as your model data is, and the problem is solved.

bennofs commented 4 years ago

This is quite old, does it have any chance to become merged in the future? Or has spec development stopped?

spullara commented 4 years ago

Many implementations have it so it is defacto in the specification. I don't think they have updated the spec itself though in a very long time.

gasche commented 3 years ago

When a parameter block passed to a parent contains tags, should those tags be interpreted in the current context, or in the context of the parameter block use-site within the parent template? I looked for a mention of this in the specification, but found none. What do current implementations do?

Consider (in the syntax of spec tests):

data: { include-site: { place: "include site"}, use-site: { place: "use site"} }
template: |
  {{#include-site}}
    {{<parent}}
      {{$var}}evaluated at {{place}}{{/var}}
    {{/parent}}
  {{/include-site}
partials:
  parent: |
    {{#use-site}}
      {{$var}}default{{/var}}
    {{/use-site}}

Should we expect evaluated at include site or evaluated at use site?

gasche commented 3 years ago

What do current implementations do?

I found a first answer in the mustache.php testsuite: for mustache.php, parameters should be evaluated lazily, in the context of the parent template.

    public function testInheritanceWithLazyEvaluation()
    {
        $partials = array(
            'parent' => '{{#items}}{{$value}}ignored{{/value}}{{/items}}',
        );

        $this->mustache->setPartials($partials);

        $tpl = $this->mustache->loadTemplate(
            '{{<parent}}{{$value}}<{{ . }}>{{/value}}{{/parent}}'
        );

        $data = array('items' => array(1, 2, 3));

        $this->assertEquals('<1><2><3>', $tpl->render($data));
    }

I wish this was made explicit in the specification -- assuming that other implementations made consistent choices.

gasche commented 3 years ago

Here is a more up-to-date list of Mustache implementations that support this extension:

gasche commented 3 years ago

Thanks for the carefully worded message! I agree that it's nice to push this further: I think that with the amount of implementations that support the feature, it's a bit silly to refer to it as a PR to the original spec.

Semantically and syntactically speaking, I'm perfectly happy with the spec as-is, and I agree that it should be merged in v2. [...]

  1. Change the spec name from inheritance.yml to ~inheritance.yml to mark it as an optional module. It complicates the implementation a bit, so it might deter new implementers otherwise.

If this is made an optional part of the spec, does it need to correspond to a major-version bump for the specification? I haven't thought hard about the versioning scheme but I would think as adding optional aspects as a backward-compatibility-preserving changes (implementations remain compliant). (One exception would be an implementation that provides a feature using the same syntax as proposed here but with different semantics, then I guess it would become non-compliant?)

  1. Make the jargon much more consistent. I found {{$block}} being referred to as "block", "parameter" and "substitution" and {{<parent}} as "parent", "super", "include" and even "partial". Especially "partial" should really be avoided in my opinion, as it is needlessly confusing. I started to make change suggestions for the incorrect appearances of "partial", but I didn't go all the way through. I would suggest using only "parent" or "super" for {{<parent}} and "parameter block" for {{$block}} when used by itself or "argument block" when used inside {{<parent}}...{{/parent}}..

I agree that the terminology could be made more consistent. I myself use parameter block for {{$foo}}...{{/foo}} (the "block" part is important because it's not just a tag, it can contain an arbitrary sub-template), and sometimes just "parameter" when the context makes it very clear. I think that "block" means generically anything with an opening and a closing form (usually {{/foo}}) and stuff in the middle.

Regarding the whole feature or {{<foo}}, I personally use the wording "partial with parameters": it behaves exactly like partials {{>foo}} (it expands to a sub-template), except it can take parameters. This is a wording that makes sense to my community (more oriented towards functional programming than oo-style inheritance), but then it is not perfect either because the shadowing semantics of partial blocks are unintuitive if you think of them as function calls (the outermost definition wins, not the innermost one as one would expect). I guess I mean that I haven't found a perfect name for this part of the feature, and we should be prepared for different terminologies to survive.

Another remark: upon implementing the feature I found that it shows the limits of the current way whitespace is handled in Mustache (the notion of standalone tags, and the indentation of partials). Ideally the user would want:

Long story short, I went down a rabbit hole of finding better specifications for whitespace handling in Mustache, and they tended to be more complex, more natural for the user, and incompatible with existing implementations / the spec. in corner cases. I gave up and went back to something mostly-standard (my implementation does substract the indentation of the parameter block, iirc.), but it remains a (minor) sticking point with the feature and its specification.

jgonggrijp commented 3 years ago

Good point about "partials without parameters" and "partials with parameters". I now realize they are more similar than I thought, and also that the implementation can probably be more similar than I thought.

If this is made an optional part of the spec, does it need to correspond to a major-version bump for the specification? I haven't thought hard about the versioning scheme but I would think as adding optional aspects as a backward-compatibility-preserving changes (implementations remain compliant). (One exception would be an implementation that provides a feature using the same syntax as proposed here but with different semantics, then I guess it would become non-compliant?)

I was thinking along these lines, too, but I didn't dare write it down. I'm glad you did. With it being an optional module, I feel it could be a minor version bump.

jgonggrijp commented 3 years ago

Since my review and previous comment, I have radically changed my opinion about jargon consistency. I now believe that "parent", "super" and "inheritance" are the needlessly confusing words that should be avoided. The thing we're talking about here is just a parametric partial. I would suggest renaming the spec to ~parametric-partial.yml. Thanks to @gasche for making me realize this.