Twig embed-like functionality could be very useful

[Onderhond]

Twig embed functionality: http://twig.sensiolabs.org/doc/tags/embed.html

Basically it's like an include, but it allows for {% block %} statements. I read through the issues here and found a similar discussion (#539) but that seemed centered around added {% block %} statement to the include function, not so much creating a new "embed" function for it.

I've been looking at various js templating languages the past week and most are seriously underpowered for what I want to do. Nunjucks is so far the only one that looks appropriate, sadly it doesn't support what twig calls "embed". I already looked around for workarounds and even though I found several options, they didn't seem very clean or comfortable to me.

The reason for including a function like "embed" is to make an abstraction of mid-level html structures. Stuff like column layouts, structural lists (the ones with sorting options and pager, not content lists), header-main-footer containers ... anything with a fixed html structure but wildly varying content. I know I could make templates for each instance of these html structures using the extends function, but then I'd have a bazillion templates I'd only be using once.

A practical example maybe: The list embed template

<section class="list {{ class }}">
  <header>
    <h1>...</h1>
  </header>
  <div class="main">
    {% block main %}
        list main content
    {% endblock %}
  <div>
  <footer>
    <div class="paging"> ... </div>
  </footer>
</section>

Using the list embed

{% embed "list" with {class: news} %}
  {% block main %}
    // put all the news items here
  {% endblock %}
{% endembed %}

Ideally I'd like the list embed to extend from a more generic header-main-footer structure template (with three blocks, one for header, one for main, one for footer) and if you're looking to reuse this template over projects you could add some if statement to include/exclude stuff like the pager).

So yeah, hope this made sense and I hope you kinda get what I'd love to do. If anything is still unclear, do ask :)

[carljm]

Yeah, I see the usefulness of that tag. Normally I'd handle those use cases with caller macros, but that only works when you have a single "block" that you want to fill in, not multiple. For multiple, I've passed in content as strings in arguments, but that's not particularly nice.

So I'm not likely to have time to work on implementing this, but if a tested and documented pull request appeared, I'd review and merge it.

[paulmsmith]

I was just about to put in a request for this. :) I'm having to do exactly @carljm's solution but it's not pretty.

[paulmsmith]

@carljm Is there a bounty thing for this repo? I'd happily pay towards this enhancement.

[carljm]

No, we don't have a bounty system.

[paulmsmith]

@carljm Thanks for the swift reply. I'll see if I can drum up some interest from people faster and better than me at Javascript.

[Onderhond]

@carljm I've been thinking about parsing the content first and passing it along as variables, but with multiple nestings (embeds in block statements of other embeds) it would become quite messy doing it that way. Also somewhat counterintuitive, since you're working inward out (starting with the smallest html components), where html is pretty much always outward in.

The beauty of the embed function would be that it would be in line with the nesting rules of plain old html. Just an extra level of abstraction, which is what a good templating lang should be imo.

@paulmsmith I'd love to help implement this btw, but I'm an oldskool html/css'er with very limited javascript experience (read simple jQuery dom manipulations and fixing everything else in css). If you can't find anyone to implement this I might ask my employer to see if he could help out a little, but no guarantees there.

[howardroark]

@Onderhond and @paulmsmith ... How about I make a fork, add you two as contributors and then we kick off a pull request. That way if anyone else passes by and likes the idea they can test and help out. I don't think this will be entirely all that simple :P

Starts here... https://github.com/howardroark/nunjucks/blob/master/src/parser.js#L1272

[Onderhond]

I'm all for it. Afraid I won't be much help on the technical side but definitely willing to bugtest when needed!

[rludgero]

Hi guys, something new about that?

Is this #794 PR ready for rebase?

i really need this enhancement.

Like @Onderhond, I also use Twig and this functionality is very useful.

Cheers!

[howardroark]

Hey @Rodrigo-Ludgero ! The fork for #794 is only just a starting place. Basically I duplicated include as embed to map out it's basic structure in the the code and ensure it's following common conventions. It still needs to be adapted to be a morphing of include and extend concepts. I added you as a contributor to the fork in case you felt like giving it a go :P

[rludgero]

Hi @howardroark , thanks for your prompt response and the invitation, but I'm afraid I will not be of much help to contribute code to the repository.

[robvenn]

Hey I'm also interested in this feature, was already adding some code without knowing a branch had been started. I started out in a similar way but assumed that embed tags always require an endembed tag to be valid, added a bunch of tests and now working on the inherited blocks...

[howardroark]

@robinvenneman Sounds better! Got a link to the code progress?

[rludgero]

Hi @robinvenneman , thanks for get to it.

In my opinion, this feature is extremely useful, increasing the ability to reuse more code and add even more power to the system template.

[rludgero]

Hi @robinvenneman , perhaps this example https://github.com/twigjs/twig.js/blob/master/src/twig.logic.js#L949 will be useful to create the tag.

[robvenn]

Hey @Rodrigo-Ludgero thanks for the suggestion... interesting to see how it's done in twig. However, I currently don't have time to work on this. I checked what's wrong with the code that I added and the embed tag works but the nested blocks are not parsed correctly from the NodeList, perhaps because it has an end tag or because I didn't use the parent template like with the extend tag. Maybe someone else can pick this up?

[howardroark]

@robinvenneman Yeah... that is kinda where I got lost too. It's certainly not as simple as just copying code from include or extend :P I suspect some refactoring would be in order.

[ArmorDarks]

Hi guys

As far as I remember, Nunjucks opted to be quite close port of Jinja2.

This means that before rushing out implementation, this issue should be elevated to Jinja2 repository first. There we should lead the talk and decide specs of this feature, ensure that it's properly fitting into Jinja ways.

While I without any doubts seeing usefulness of this addition, I have some doubts in current implementation. embed seems to me like a mix between macro and React component, but in current implementation it seems to be more a synergy of include and something else. It might be more logical to enhance macros instead and allow them to work more like a component, and to use more powerful callbacks instead of a caller(), which would be able to uphold even blocks. This might give us in return extremely wider flexibility.

Anyway, this is something that should be discussed with Jinja2 team first. There we'll be able to get better feedback.

[internalfx]

I have been working on implementing this for a day now...going into day 2.

[internalfx]

I'm currently stuck in the compiler....

I have gotten the template loaded, and I have the blocks. But I'm having a hard time getting the block content from the template that called embed and pushing it into the loaded template.

[internalfx]

it looks like context.addBlock() just pushes the block onto an array.

I suppose that is how the renderer keeps track of which content to use.

digging further....(man this rabbit hole is deep)...

[internalfx]

ok, here is my current parseEmbed implementation....

    parseEmbed: function() {
        var tag = this.peekToken();
        if(!this.skipSymbol('embed')) {
            this.fail('parseEmbed: expected embed', tag.lineno, tag.colno);
        }

        var args = this.parseSignature(true, true).children;
        this.advanceAfterBlockEnd(tag.value);

        var node = new nodes.Embed(tag.lineno, tag.colno);
        node.template = args[0]
        node.contextVar = null

        if (args.length === 2) {
          node.contextVar = args[1]
        }

        node.body = this.parseUntilBlocks('endembed');
        this.advanceAfterBlockEnd();

        return node;
    },

node.body now contains the declared blocks to inject into the embedded template.

[internalfx]

I recently discovered that nodes have a findAll function.

In my compile method I am able to get the blocks in node.body with this...

node.body.findAll(nodes.Block)

getting closer....

If there are any maintainers here, feel free to chime in with pointers.

[internalfx]

:dizzy_face:

I feel like I'm gonna have to modify the render method itself.

I simply can't seem to find a way to get the parents template blocks to override a childs.

[paulmsmith]

@internalfx Loving the updates. 👍

[internalfx]

Finally got it working....

first working version

https://github.com/internalfx/nunjucks/commit/e8f6ab1e719450e612b7a9290bb79e9f3ea5a1e2

[rludgero]

@internalfx great job 💻 💾 , thank you very much for having begun to create the tag. 👍 👏

[internalfx]

I have merged the latest from upstream.

I could really use some testers...

You can run my version by modifying your package.json like below.

"nunjucks": "internalfx/nunjucks"

then npm install nunjucks

[robvenn]

Awesome, can't wait to play around with this! Great job @internalfx 👍

[internalfx]

Fixed a few bugs, getting more stable...

[paulmsmith]

Nice! Is it in a state that @carljm can review now do we think?

[internalfx]

PR #814 submitted.

[carljm]

@ArmorDarks You're right that I've tried to keep nunjucks from diverging too much from Jinja2. This is particularly true when it comes to making changes that are simply incompatible with Jinja2 (i.e. such that it would no longer be possible to share template files between nunjucks and Jinja2). However, I'm not so opposed to adding new features in nunjucks that don't exist in Jinja2, if there's a good rationale for them, and they don't break compatibility (e.g. this is just a new tag, people who want Jinja2 compat can just not use it). There are already features in nunjucks that don't exist in Jinja2 (e.g. async rendering). And while anyone who wants to propose this feature for Jinja2 is welcome to do so, I don't want to make acceptance into Jinja2 a blocking requirement for acceptance into nunjucks.

Do you have concerns about this feature specifically that we should discuss?

@internalfx thanks for all your work! I'll take a look when I can.

[paulmsmith]

@carljm I think that's a really nice stance on it. I completely agree that diverging Jinja2 features would be bad but supplementing Nunjucks with useful features is a good thing.

[ArmorDarks]

@carljm I get your point.

My main objective concern is future compatibility with Jinja2. embed is no small addition to Nunjucks, and it might greatly affect how templates would be written. Future, in which Jinja2 will enroll some addition, and we won't be able to implement it because it will break embed or overlap with it seems to be quite grim. While we're rushing here with implementation, we might simply oversee obvious pitfalls, where embed doesn't fit well in current Nunjucks environment. I think design should be thoughtful and solid, consistent.

That's why I propose at least to open issue in Jinja2 repository. If they will say "that's nice, but we're not going to implement it" — that's okey. We can extend safely. But what if they will say "oops, that won't work with certain Jinja2 features"? We shouldn't forget that Nunjucks still have a lot of not implemented Jinja2 features, and we don't take into account them.

My subjective concern here is a strong feeling that something is wrong, like there are some nasty overlaps between embed and macro. However, I can't object, because so far I didn't come to better overall design myself, didn't come up with solution, which will solve that issue better. Once again, opinions from third party (Jinja2 maintainers) might give us some fresh thoughts too...

But I do not object against your decision. You're the boss, after all :)

[internalfx]

What is the use case for Jinja2 compatibility? For me it was because Jinja2 had a nice syntax, but being able to run on node or python was irrelevant.

Are people really running the same templates on two platforms simultaneously?

How often are people really switching platforms. Probably at most once, if ever.

Sounds like compatibility for the sake of compatibility to me.

[Onderhond]

First of all, thanks a bunch for all the hard work! Really looking forward to start testing the code.

As for compatibility and switching platforms, my reason for asking about the embed is because at the company I work for we'll be alternating between node.JS and Laravel solutions (headless Drupal as CMS). Having a templating middle ground that's ~95% compatible (Nunjucks - Twig) is a HUGE bonus for us. It means minimal work to maintain two sets of components. So yeah, there are definitely use cases there :)

[internalfx]

@Onderhond That raises another issue as well, you are wanting compatibility with Twig. @ArmorDarks Wants compatibility with Jinja2.

Twig is already quite diverged from Jinja2 as it is.

I added embed because I'm working with another developer who uses Twig often. He was wondering if nunjucks had an equivalent to embed. Having never used Twig I'm wondering "what's embed?". When He showed me what it was and what you could use it for I immediately decided that Nunjucks needed the same functionality.

It wasn't about compatibility with Twig. It was about how cool embed was.

[internalfx]

My vote is that if compatibility can be maintained without sacrificing features, then it's fine to keep compatibility.

If it gets in the way of innovation, it's gotta go.

My 2 cents. (I am a bit of a "move fast and break things kinda guy")

[rludgero]

It wasn't about compatibility with Twig. It was about how cool embed was.

Embed saves a lot code.

This functionality is what stands out above all others templates engines I've used.

And yes, embed is very cool!

@internalfx, once again, thank you for using your time in creating this tag.

[carljm]

@internalfx Yes, sharing templates between two platforms is a real use case, and one I'm absolutely not going to sacrifice. For me personally, the only reason I ever picked nunjucks in the first place was because it allowed sharing the same templates between Python server and JS client; every project I've used nunjucks on did this. It's hugely valuable to be able to do that.

[internalfx]

To everyone who has appreciated the work on the embed tag, you are quite welcome, enjoy.

@carljm We clearly have different priorities in development, but in this case it doesn't matter.

embed does not break any compatibility that I can see, it only enhances it's compatibility with Twig.

Everyone gets what they want. (except @ArmorDarks I guess)

[howardroark]

If we get jinga to accept a PR, would we be able to merge this?

[Onderhond]

@internalfx the compatibility is a bonus for us. It's important, but the actual embed functionality is the part that is crucial of course. I know there's already some divergence between nunjucks/twig/jinja2, but so far the changes are quite minimal and most importantly, manageable.

In the past we used a custom templating language (xml-based) exactly because templating languages 7-8 years ago typically didn't have the embed functionality (mind you, this was for generating static templates, not live code). If you want to maximize reuse of code, it's an essential tool imo. So again, kudos for putting in the hard work!

[carljm]

I already said above that this feature doesn't break compatibility with jinja2, and I'm not going to block it on a matching feature in jinja2. It's just waiting on me (or one of the other maintainers) having time to review.

@ArmorDarks I'm open to more discussion, but for that to be useful I think you'll have to be able to put your concerns into more concrete form, with a proposed alternative.

[carljm]

@ArmorDarks you're also free to open an issue on jinja and get their feedback on the idea. You can point to the fact that Twig (based on jinja2) decided to add it.

[ArmorDarks]

That would be quite weird to make an issue to Jinja2 repository by person, which has been quite doubtful in this feature consistency, I think.

I don't have any meaningful proposals for now, and I don't want to be a douchebag which only criticizes and blocks all other people work. In fact, I never intended to. I only had some worries regarding possible collisions with Jinja2 maintainers view. In all others senses, I'm not opposed to embed functionality and see it as welcome addition to Nunjucks.

[allmarkedup]

Really looking forward to seeing this getting merged in and released 👍 Twig-style embeds are a massive help when developing templates for component/pattern libraries and this would really make Nunjucks a much more useful tool for this purpose. Great work @internalfx.

[allmarkedup]

@internalfx Just a heads up - I'm just testing this out using your fork and it seems to be throwing an error when I try and specify context data using with. This works:

{% embed "panel" %}
    {% block content %}
    Panel content
    {% endblock %}
{% endembed %}

This throws an error for me (parseSignature: expected comma after expression):

{% embed "panel" with { foo: 'bar' } %}
    {% block content %}
    Panel content
    {% endblock %}
{% endembed %}

I haven't dug into it to try and track down the problem but it's probably worth seeing if you can replicate - assuming with support is part of your PR?

[internalfx]

@allmarkedup context data not supported.

nunjucks doesn't seem to have lexing for arguments like that.