Verify maturity of Twig engine

[geoffp]

@sghoweri's Twig engine is merged to pattern-engines, with some tweaks and simple unit tests. @EvanLovely has also expressed interest in the Twig engine; Evan, would you be willing to give it a whirl and see what issues come up?

We're not doing anything currently (that I know of) to catch the JSON context directives in Twig template includes, such as with and only. I believe @sghoweri knows about this issue in more detail. Or was that the kind of thing that expandPartials: true solved?

[EvanLovely]

I'm taking a look at stuff right now! I've put up a quick PR for some initial fixes here: #286

[EvanLovely]

OK, my first pass shows good and expected behavior with:

• rendering basic twig templates from global data and having a sidecar json file override the data
• including a twig file in another
• including a twig file in another and having the sidecar json file override the included files data

However, I found strange behavior when using extend: it acts like an include! Further digging leads me to finding that this line appears to be looking for include, extend, or embed to use in the expandPartials functionality.

This worries me as the Twig docs claim that extends and blocks that make up Template Inheritance are the most powerful part of the language. I'd rather see a compile fail when extends are present than subtly different and unexpected behavior.

Planning on doing some more digging soon; really excited about this though!!

[sghoweri]

Interesting. I'm wondering if the expandPartials flag is the cause for this unexpected behavior?

For a bit of perspective:

Earlier iterations of the Twig engine involved modifying the object factory's render function to include a new parameter pulling in "this", the all encompassing data object which includes the absolute path of each individual Twig template that's being rendered (which wouldn't have been available otherwise).

By having that absolute path on hand when it came time to render the different Twig partials (includes, embeds, and extends), I was able to include this parameter into Twig.js to resolve paths correctly.

HOWEVER, going down this more manual route (not using the extendPartials flag) resulted in me not being able to natively use PL's shorthand path syntax (in the things getting pulled in if I remember property?)

FWIW, with a bit of regex and time, I was able to more or less recreate most of the same shorthand syntax behavior using just the Twig.js and Gulp Data plugins (completely separate from Pattern Lab -- not even in a project that was using PL), however I didn't get far enough with this to actually explore if this approach was necessary in this particular instance, given that Pattern Lab already handles so much of this for us already.

Keep me posted on where these tests net out. This is a crazy busy week for me but I'll do what I can to help support this (including anything code-wise through the weekend if need be). This is exciting!!

On Mar 9, 2016, at 9:03 PM, Evan Lovely [email protected] wrote:

OK, my first pass shows good and expected behavior with:

rendering basic twig templates from global data and having a sidecar json file override the data
including a twig file in another
including a twig file in another and having the sidecar json file override the included files data
However, I found strange behavior when using extend: it acts like an include! Further digging leads me to finding that this line appears to be looking for include, extend, or embed to use in the expandPartials functionality.

This worries me as the Twig docs claim that extends and blocks that make up Template Inheritance are the most powerful part of the language. I'd rather see a compile fail when extends are present than subtly different and unexpected behavior.

Planning on doing some more digging soon; really excited about this though!!

—
Reply to this email directly or view it on GitHub.

[geoffp]

I think that's right -- it looks like the partial finding regex is flagging extends and embed as partial calls, which may not be strictly correct:

/{%\s*(?:extends|include|embed)\s+('[^']+'|"[^"]+").*?%}/g

The behavior of expandPartials is to sort of "manually" replace a partial call with the string result of rendering that partial, which means that Twig itself is no longer actually involved in rendering partials. As far as it knows, it's just rendering a bunch of individual templates; PL sees a partial call, and with regexes, goes and finds the template it requests, calls Twig to render the template, and replaces the partial call with the string result.

This is probably not what we want for extends and embed.

With Handlebars, we need to not do this. Handlebars wants you to "register" every callable partial (by whatever name you want) so it can keep track of them internally, which makes supporting pattern keys really easy. We do that with a hook in addPattern() in pattern_assembler. I was hoping there was some way of doing the same thing with Twig.

[geoffp]

The first prerelease of the Pattern Lab 2.X port of the Twig engine is here: https://github.com/pattern-lab/patternengine-node-twig

[bmuenzenmeyer]

Will be checking this out next - most of the PR backlog is done

[sghoweri]

@bmuenzenmeyer I might be able to help out a bit with this one...

A couple of weeks ago I started playing around with Pattern Lab Node to try to see how viable it might be to get Pattern Lab PHP 2 for Twig and PL Node to share the same internals (thanks to node-twig and some inspiration from this Fractal + PHP Twig project. Also tried getting the original Twig.js engine working as well (which I recall working with @geoffp on a bit a couple years ago) just to see what would happen.

At first glance, I ended up running to a bunch of issues getting the Styleguidekit Twig Default templates to be used instead of the default Mustache templates, hard coded references to those default templates (which I had to swap out for references to the engine being used), etc.

Those were pretty manageable to work around but I recall hitting a bit of a blocker relating to Twig needing to know the path of a template being rendered (or something like that) which I didn't get a chance to look further in to...

Needless to say - I might be able to chip in a bit on this one.

CC @EvanLovely

[bmuenzenmeyer]

@sghoweri thanks for the notes Salem! Glad to have your voice back into this side of the project.

The templates issue is precisely why I want to allow users to specify their templates in a more flexible fashion that what we offer today - which is a combination of the paths specified in https://github.com/pattern-lab/edition-node-gulp/blob/master/patternlab-config.json and some hard-codes inside https://github.com/pattern-lab/patternlab-node/blob/master/core/lib/patternlab.js#L512 which target the default-styleguidekit too tightly

shouldn't be too much effort on that front, but any help on the Twig engine would be much appreciated!

cc @geoffp

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[EvanLovely]

Ha! I kinda love this cause we got pinged collectively on the start of this issue nearly two years ago @sghoweri !

[renestalder]

Would really love a move here.

If you need some one to help verify: I have a very huge patternlab library running on PHP Twig currently with strict_variables Twig option enabled. It uses quite a big range of Twig features including Macros, Embeds, Includes,...

Two things, separate from each other I have on my list for a long time:

• Upgrade to Twig 2.0 (Patternlab PHP Twig is still on Twig 1.0).
• Migrate to Node, thus Patternlab Node Twig.

So let me know if you need someone helping to find bugs. I then would try to migrate. Sadly I couldn't give you access to the code because it's a client project. But I can provide tests.

[ringods]

I would very much like to see the engine-twig package be updated to use twig, a 100% pure Javascript implementation of the twig pattern language and actively maintained (last release 20 Feb 2020).

Contrary, node-twig hasn't had a release in the past 2 year.

The README file of engine-twig points to 2 github issues: this one and #554.

This issue discusses the maturity of the Twig engine. With a switch to the twig package mentioned earlier, this engine could get a huge update.

Issue #554 is about the support for extends. The implementation notes of twig indicate that extends is supported.

Would you welcome a pull request which tests @pattern-lab/engine-twig with the twig JS library?

[bmuenzenmeyer]

Would you welcome a pull request which tests @pattern-lab/engine-twig with the twig JS library?

I would certainly. I'd be relying on you and other users to verify the functionality within your mature PL's however. Long term there has been discussion of deprecating with this engine in favor of engine-twig-php - but that won't happen overnight. We could even work out something where engine-twig becomes a community-maintained repo.

[ringods]

@bmuenzenmeyer ok, on it in #1135

Hello @renestalder, are you able and do you have time to test my work against your extensive twig pattern library? If so, let's discuss in the pull request.