Accessing template variables without $this

reinink commented 10 years ago

This request keeps coming up:

Can we access template variables without the $this pseudo-variable?

As discussed in #9, this is not possible with template functions, and therefore it seemed unwise to do this with template variables (as it would create inconsistency between how these two are accessed).

Further, prior to the changes made to nesting and layouts, accessing variables without $this wasn't even possible. However, because nested templates now have their own variable scope, this may actually be be possible...with some caveats to sections. Consider this example:

<?php $this->start('content') ?>

    <h1>Welcome!</h1>
    <p>Hello <?=$this->e($this->name)?></p>

<?php $this->end() ?>

The contents of this section will be placed into the variable <?=$this->content?>. Generally, sections are used with inheritance, in which case there shouldn't be an issue. The section variables can be extracted before the layout is rendered, making the content available in the layout's local scope ($content).

However, if you want to use this variable in the same template, this will not be possible. For example:

<?php $this->start('content') ?>

    <h1>Welcome!</h1>
    <p>Hello <?=$this->e($this->name)?></p>

<?php $this->end() ?>

<!-- This will not work: -->
<?=$content?>

There are two ways around this. You could access this variable the old way (<?=$this->content?>), but that now creates an odd and very inconsistent approach to accessing variables. Alternatively, section variables could be accessed using a new template function, such as: <?=$this->section('content')?>. I'd be inclined to say, for consistency's sake, that all section variables should be accessed using this new section() function. For example:

<html>
<head>
    <title><?=$this->title?></title>
</head>

<body>

<div id="content">
    <?=$this->section('content')?>
</div>

<?php if ($this->section('sidebar')): ?>
    <div id="sidebar">
        <?=$this->section('sidebar')?>
    </div>
<?php endif ?>

</body>
</html>

With the sections issue out of the way, does it now make sense to encourage (force) everyone to access variables without the $this pseudo-variable? We could theoretically allow both approaches, and developers can choose which one they prefer. Personally, think that is a bad idea since it creates inconsistency in the templates. I'd prefer to simply choose which approach is better, and force that. This would clearly be a breaking change, meaning a version bump (3.0).

So, is everyone okay with having template functions accessed differently than template variables?

<h1>Hello, <?=$this->e($name)?></h1>

anlutro commented 10 years ago

Making it work with nested layouts seems like the tricky part.

reinink commented 10 years ago

If by nested layouts you mean stacked layouts, it actually works without any issue. All layouts are rendered in the same scope as the template, so if one layout has access to a variable, they all do.

anlutro commented 10 years ago

Ah, I was working with the constraint of only the variables set after calling $this->layout() would be available for the parent layouts.

anlutro commented 10 years ago

I've messed around a bunch as the whole templating engine thing has caught my interest lately. If you're on the path of deprecating old behaviour, how about deprecating the $this in favour of something like $t, which would make it far easier to isolate the actual template file from the template class, and prevent access to internal properties altogether? It would entail replacing the include() call with a call to something like this:

protected static function includeTemplate($t, $__path, array &$__vars)
{
    extract($__vars, EXTR_REFS);
    unset($__vars);
    require $__path;
}

Furthermore, the easiest way I could find to create several layers of layouts is to do $this->layout = new Template(...) and somehow ensure that $this->variables carry over to the layout template, pass any blocks defined by the template to the layout on render, and then return $this->layout->render() from the template's render call.

reinink commented 10 years ago

@anlutro I don't see the internal variables as an issue. Simply having one reserved $internal variable really makes things nice and tidy. Further, by dropping $this from template variables, you can actually still use $internal in your templates, no conflicts whatsoever!

Further, I've actually got locally scoped variables (no $this) already working in Plates (locally), without causing any issues with the layouts (or stacked layouts). The only change required was accessing section content using a new function called $this->section('name'). Here is a full example:

Within your controller

<?php

// Create new Plates engine
$templates = new \League\Plates\Engine('/path/to/templates');

// Create a new template
echo $templates->render('profile', ['name' => 'Jonathan']);

The page template

<?php $this->layout('template', ['title' => 'User Profile']) ?>

<h1>User Profile</h1>
<p>Hello, <?=$this->e($name)?></p>

<?php $this->start('sidebar') ?>
    <ul>
        <li>Sidebar</li>
        <li>List</li>
        <li>Goes</li>
        <li>Here</li>
    </ul>
<?php $this->end() ?>

The page template

<html>
<head>
    <title><?=$title?></title>
</head>

<body>

<div id="content">
    <?=$this->content()?>
</div>

<?php if ($this->section('sidebar')): ?>
    <div id="sidebar">
        <?=$this->section('sidebar')?>
    </div>
<?php endif ?>

</body>
</html>

reinink commented 10 years ago

Initial work on this committed (fb5e204573bbc14a320e8be922373ddfd9ebee07). If this change interests you, please download (using dev-master) and give it a try.

lekoala commented 10 years ago

I think it's great to allow accessing variables without this. After all, this is the point of the short opening tags : less typing for templating. The only thing strange here in my opinion is this line:

<?=$this->e($name)?>

There is no real benefits of having variables extracted if you need to do that to have secure templates. Alternatives are:

Don't extract variables and call variables with $this->e('name') (actually, little difference in terms of typing)
Allow to automatically escape variables before extracting them (optional, can be turned on or off) which allows <?= $name ?> securely
Use stream wrappers in a zend_view fashion. It involves a little bit of parsing, but it's still pure php and it could convert <?= $name ?> to <?= $this->e($name) ?> transparently. Or just add the <?= $this-> in front, so we could do <?= e($name) ?> (I think it's possible, to be confirmed). We can limit the "magic" to short tags to keep things neat.

I know that automatic escaping is a pain in pure php template but I think it's worth considering.

anlutro commented 10 years ago

The problem with pure PHP templates is it shares the global namespace, so you can't, for example, define function e($string) but make sure that it's only available to the template. With a pure PHP templating engine there's really nothing stopping you from doing mysql_connect if you absolutely wanted to, which is why $this->foo() is used.

lekoala commented 10 years ago

if you define a function inside a namespace you don't have the problem of a globally defined function. And the approach I was suggesting was to use a stream wrapper to replace e() with $this->e().

anlutro commented 10 years ago

Missed the stream wrapper part. That's really interesting.

reinink commented 10 years ago

Don't extract variables and call variables with $this->e('name') (actually, little difference in terms of typing)

To be clear, the template function $this->e() simply exists for escaping variables. Escaping is a "necessary evil" of pure PHP templates. That has nothing to do with extracting variables—both locally and object scoped variables must be escaped.

Allow to automatically escape variables before extracting them (optional, can be turned on or off) which allows <?= $name ?> securely

Auto escaping variables is something I've looked very closely at, and decided against. You can read why in this pull request.

The issue isn't getting the variable automatically wrapped with an escape function, the REAL issue is dealing with objects and arrays, which are MUCH more difficult to handle, as I describe in that PR. In fact, AuraPHP had automatic escaping build into version 1 of the View library, and have actually removed it from version 2 for these very reasons.

if you define a function inside a namespace you don't have the problem of a globally defined function.

Using namespaces would be great, but I see two issues. First, you would have to define the namespace at the top of each template, which is ugly. Second, you cannot alias (use) functions in PHP until version 5.6, meaning you would have to write the fully qualified namespace for each function, which would be terrible. I do potentially see some changes to Plates when PHP 5.6 has been widely adopted, but that isn't going to be anytime soon.

And the approach I was suggesting was to use a stream wrapper to replace e() with $this->e().

This is a very interesting idea, granted it introduces a parsing stage, which I really do not like. However, this approach could theoretically solve two issues:

It would automatically wrap all echoed data with $this->e() (auto-escaping just on outputted data)
- Automatically changing short codes (<?=$name?>) would be easy, but any more complicated echoing could be problematic (<?php echo $this->first_name . ' ' . $this->last_name; ?>).
- How would you handle outputting of variables that should not be escaped?
It could automatically add $this-> to all function calls
- There is the risk of conflicts between Plates' functions, and any other helper functions that may exist in a project.

I believe the primary reason people like native PHP templates is because they want to work with straight PHP. They know the language, and they know the functions available to them. No need to learn another syntax (ie. Twig). However, the speed benefits do exist as well. Introducing a parsing stage will slow Plates down. Would people still use it? Or MAYBE this is exactly what they want? Straight PHP templates, with a little parsing to make their lives easier. I just don't know.

reinink commented 10 years ago

Reference: /Zend/View/Stream.php

It appears the primary purpose of the class is to convert <?= ?> to long-form <?php echo ?>.

Zend openly cautions against using this technique:

Usage of the stream wrapper will degrade performance of your application, though actual benchmarks are unavailable to quantify the amount of degradation. We recommend that you either enable short tags, convert your scripts to use full tags, or have a good partial and/or full page content caching strategy in place.

I also found this article, which basically outlines exactly what we're suggesting for automatic escaping.

anlutro commented 10 years ago

I think the parsing is a pretty necessary evil, and as long as you cache the parsed templates it really is not a huge deal in my mind.

I think the main benefit of a "pure PHP" templating engine is that it allows and encourages plain PHP syntax but builds on top of it. Laravel's Blade is a decent example of this. Whatever you put inside {{ }} will be parsed as regular PHP so you know what's going to happen with it. Latelty there have been some changes moving away from this, such as {{ $foo or 'bar' }} which turns it into a ternary operation using isset(), while <?php echo $foo or 'bar'; ?> is still valid PHP.

Compare that to Twig where something as simple as a foreach loop adds 6 local variables, and to achieve the object.property notation, Twig_Template::getAttribute() is called on runtime every time, which has like 5 different strategies for actually getting the attribute. This is a massive amount of abstraction and obfuscation that can make debugging and profiling more difficult.

As for the view stream reference, I think the example given in this article which replaces @$var with $this->var using a simple str_replace is far more interesting.

reinink commented 10 years ago

If we did go down this road, I wouldn't want to introduce any new syntax—it should be 100% valid PHP. At least that's how I see it at this moment.

The use of @ for escaping is cleaver, but it's really a hack, and certainly not the intention of this error control operator. I'm inclined to say that only variables outputted using short tags (<?=$name?>) having auto escaping applied, where all other outputted variables require manual escaping. So, if you wanted to output some HTML, you would simply do this: <?php echo $html ?>.

I'm also wondering what approach is better: streams or caching. Streams allow you to do all this work on the fly, but this clearly comes with a performance hit. Saving the parsed templates to a cache folder seems like a smarter approach—and is actually why Twig is as fast as it is. Once the parsed template has been cached, it really is just straight PHP.

reinink commented 10 years ago

Another reference: Pike_View_Stream

In particular, their auto escaping method:

public static function autoEscape($data)
{
    // Convert "<?" to "<?php"
    $data = preg_replace('/<\?(?!xml|php|=)/s', '<?php ', $data);

    // Convert "<?php echo $var" to "<?php echo $this->escape($var)"
    $data = preg_replace('/\<\?php\s*\n*\s*echo\s*\n*\s*(.*?);*\s*\n*\s*\?>/',
        '<?php echo $this->escape((string)$1); ?>', $data);

    // Convert "<?= $var" to "<?php echo $this->escape($var)"
    $data = preg_replace('/\<\?\=\s*\n*\s*(.*?);*\s*\n*\s*\?>/',
        '<?php echo $this->escape((string)$1); ?>', $data);

    // Convert raw value that are defined as "<?=~", but are converted with the line above to
    // "<?php echo $this->escape(~$var)". Convert these cases to "<?php echo $var"
    $data = preg_replace('/\<\?php\s*\n*\s*echo\s*\n*\s*\$this-\>escape\(\(string\)~(.*?)\);*\s*\n*\s*\?>/',
        '<?php echo $1; ?>', $data);
    $data = preg_replace('/\<\?=~\s*\n*\s*(.*?);*\s*\n*\s*\?>/',
        '<?php echo $1; ?>', $data);

    return $data;
}

lalop commented 10 years ago

That is really interesting, I think that could be a real great enhancement of Plates. I want to suggest 2 tips:

auto-escape should be in configuration (true/false)
we could have 2 methods to escape or not the value (e/raw), I'm not fan of the @ way which one is clearly smart but open new issue, and the ~ one

Implementing that via a plugin will set Plates as a high customizable state.

reinink commented 10 years ago

Folks, I accidentally deleted my last (long) response here. If anyone got a copy via email, can you post it back in here? Thanks!

reinink commented 10 years ago

@lalop Thank-you very much. I'll reformat and maybe post this to a new ticket. This discussion is now gone far away from simply removing $this from template variables. I'm really interested in knowing what those already using Plates think of this change, and having a clean issue is a nice place to do this.

lalop commented 10 years ago

You're welcome :)

reinink commented 10 years ago

I've continue the parsing discussion here: #23

reinink commented 10 years ago

As of fb5e204573bbc14a320e8be922373ddfd9ebee07, accessing template variables without $this is now possible. Going to close this issue.

thephpleague / plates