Page MenuHomePhabricator

Mustache templates in core do not support partials (client side)
Closed, ResolvedPublic


This is a bit of an oversight... and one of the most important and useful parts of a templating library :)

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: UI-Standardization.
Jdlrobson added a subscriber: Jdlrobson.

Change 206490 had a related patch set uploaded (by Jdlrobson):
Improve Mustache Templating offering

Jdlrobson renamed this task from Mustache templates in core do not support partials to Mustache templates in core do not support partials (server side and frontend side).Jun 13 2015, 1:05 AM
Jdlrobson set Security to None.

The Blueprint skin uses a simple partial. Converting to core's server-side HTML templating
(see gerrit 223165) results in error:

Error: Can not find partial file for 'sidebar', you should set correct basedir and fileext in options

It seems the fix could be as simple as passing a 'basedir' => $this->templateDir to LightnCandy::compile in TemplateParser.php.

@Krinkle questions in gerrit 206490 whether we should support partials:

needs... discussion about whether this should be enabled. From what I remember, there was agreement against this. Both for security reasons and for DOM integrity. These can be parametrised though, and that way a future implementation can allow data binding.

But our HTML template docs say "Templates should follow the Mustache-5 specification", and describes partials.

I don't remember this "agreement". My understanding was we said no to mixins but not partials so I think/hope the lack of partial support is an oversight (if not I think what we have in core is very half baked).

I'm not sure what security/DOM integrity issues the support of partials causes - a partial is just a way of splitting a large template into smaller templates.

Change 223218 had a related patch set uploaded (by Spage):
Support mustache partials in server-side templates

Change 223218 merged by jenkins-bot:
Support mustache partials in server-side templates

Should this task be closed as the patch has been merged?

TTO added a subscriber: TTO.

Not convinced that this blocks 1.26 release, it seems like an internal change.

@TTO: Releasing 1.26 with incompatible client and server-side template support seems a bit messy. Jon and I have pinged @Krinkle numerous times since April (when he -1ed it) to re-review, but haven't heard anything back. I would still love to get this merged for 1.26 if possible, but I agree that it's not really a blocker.

kaldari changed the task status from Open to Stalled.Dec 2 2015, 2:14 AM

Changing status to stalled as we're still waiting on a re-review from @Krinkle. I think there was a degree of misunderstanding about what level of support for Mustache we wanted to implement. My understanding is that we would implement the minimum Mustache-5 specification, as mentioned in the original RFC and similar to what has been implemented in MobileFrontend for the last few years. No custom functions or handlebars extensions would be supported in order to keep the templates simple and secure. Partials, however, are not an add-on to Mustache, but a basic and integral part of the specification. We support them on the server side and we support them in MobileFrontend. Supporting them on the client-side is essential for being able to share templates between the client and server-side, as well as moving some basic mobile functionality out of MobileFrontend and into core. So far, I haven't heard any specific arguments about why we shouldn't enable partials on the client side, other than vague concerns about security, performance, and DOM integrity. None of those have been issues in the several years that partials have been supported in MobileFrontend, however.

kaldari renamed this task from Mustache templates in core do not support partials (server side and frontend side) to Mustache templates in core do not support partials (client side).Dec 2 2015, 2:15 AM

Change 206490 merged by jenkins-bot:
Support Mustache partials in Mustache template module

Jdlrobson claimed this task.