Skip jinja2 macros to build filter SSR expressions

matthiasrohmer commented 4 years ago

Our custom Markdown tag [filter] currently relies on one unnecessary transpiling step. The flow is as follows:

We have [filter *] in our Markdown which get's rewritten to jinja2 calls like {% call filter * %} which then get evaluated to SSR statements like [% if format.includes(... %].

Instead of relying on the template engine to build the SSR statements our Markdown extension should directly emit them.

sebastianbenz commented 4 years ago

Maybe I'm seeing things to simplistic, but couldn't we define custom Nunjucks tags that match our filter syntax?

[% format "emails" %]
may the force be with you
[% endformat %].

This might require rewriting the existing docs, but that'd be fine imo.

matthiasrohmer commented 4 years ago

Yes, that would work for the general functionality. Two things to consider:

1) We'd still need a way to parse the filtering in the Grow-layer to format-filter the TOC. Alternatively, we could also generate the TOC in our SSR-layer. 2) I only thought about solutions that work for all of our tags ([tip], [video], [filter]) - for the tags that involve templating the SSR approach feels a little wrong as it would really mix things up.

sebastianbenz commented 4 years ago

TOC in SSR is probably going to be ugly. Probably not uglier than in Grow though.
Can you elaborate?

matthiasrohmer commented 4 years ago

Sure! For [filter] the proposed approach works fine, since it doesn't create any new HTML but only works with the already existing structure.

I'll try to illustrate what I don't like about SSRing for example the [tip] shortcode. That's the flow currently:

---
$title: Document
---

# Headline
Lorem ipsum.

[tip type="note"]
That's a note.
[/tip]

This data file gets rendered by Grow in two phases. The first one is the markdown render step in which we also transform our custom markdown blocks to {% call %} blocks:

<h1>Headline</h1>
<p>Lorem ipsum.</p>

{% call tip(type='note') %}
That's a note.
{% endcall %}

The second one is jinja2 rendering all remaining jinja2 statements: {% call %}, '{% include %}`, ...

<body>
  <h1>Headline</h1>
  <p>Lorem ipsum.</p>

  <div class="tip">
    <p>That's a note.</p>
 </div>
</body>

The result of this last phase is our final document in all its glory. There currently is no other process that alters the actual structure - besides SSR statements cutting out certain parts of it and the AMP optimizer.

If we use the same approach that works for [filter] for [tip] the last rendering step during build wouldn't produce a final document but:

<body>
  <h1>Headline</h1>
  <p>Lorem ipsum.</p>

  [% tip type="note" %]
  That's a note.
  [% endtip %]
</body>

But different than the proposed [% filter %] the [% tip %] tag would need to render new HTML. To be specific the tip.j2.

I know that is not really rational reasoning nor something that strictly speaks against doing it this way. For me it just feels as it would mix things up that we haven't mixed so far 😉

sebastianbenz commented 4 years ago

Thanks for the detailed write-up. I see your point and I agree that this is something that should idealy be handled as part of the markdown transformation. We should also keep the syntax consistent.

ampproject / amp.dev

Skip jinja2 macros to build filter SSR expressions #3657