vlucas / bulletphp

A resource-oriented micro PHP framework
http://bulletphp.com
BSD 3-Clause "New" or "Revised" License
418 stars 50 forks source link

Templates are rendered twice #35

Closed ellotheth closed 10 years ago

ellotheth commented 10 years ago

Response::content() sets or retrieves stored rendered response content. Template extends Response and overrides content(), but without any internal storage--the template content is rendered every time content() is called.

In the Bullet app flow, content() is initially called in the Bullet response handler (App::_handleResponse()), which checks to see if the response should be rendered as JSON. content() is called again via Response::__toString() when App::run() is printed. For Template responses, that means the full template (including the layout) is rendered at least twice (more if more response handlers are registered).

index.php

<?php
global $tpl_counter; // tracks how many times the template is rendered
$tpl_counter = 0;

require_once dirname(__DIR__).'/vendor/autoload.php';

$app = new \Bullet\App(array(
    'template.cfg' => array('path' => dirname(__DIR__).'/src/')
));

$app->path('/', function($request) use ($app) {
    $app->get(function($request) use ($app) {
        return $app->template('template');
    });
});

echo $app->run(new \Bullet\Request());

template.html.php

<?php
global $tpl_counter;
$tpl_counter++; // increment the counter for each each template rendering
?>
content in the template: <?php echo $tpl_counter; ?>

page output

content in the template: 2

A possible solution could be to use the internal content storage in Template, so calling Template::content() only renders new content if the internal storage is empty. Alternatively the response handlers could skip any Template-type responses.

vlucas commented 10 years ago

Wow, fun times. I love how programming makes you feel awesome, and then completely dumb just a moment later. I'll get this fixed :)

ellotheth commented 10 years ago

Hey, we found this because we failed at implementing Iterator. Good times all around!

preinheimer commented 10 years ago

as long as we don't talk about the literal hours I spent trying to dig into this before I handed it off to Gemma in a huff, I'm fine.

vlucas commented 10 years ago

This bug actually bit me 2 days ago, but I never resolved the problem or realized why it was happening. I was trying to display flash messages in a layout view if they existed and then clear the messages from session after printing them out, but they were never showing up. I would print them and then die in the view, so I knew they were there, but they would never display without me killing the process when I let the render process run. Now that I know the view was double-rendering, I know exactly what was happening. Groan. Thanks for catching this, @ellotheth.

ellotheth commented 10 years ago

It was a great case study for not chucking database logic in your views, though. "Why do I have two rows for everything?!"

Keep things decoupled OR ELSE!

preinheimer commented 10 years ago

For me the key WTF thought was putting something in a loop in the template that sent out an HTTP header, seeing that header with the response, but not seeing the echo that was on the preceding line. This because the loop worked the first run through the template (when the contents were ignored) but not the second. @ellotheth saved me

vlucas commented 10 years ago

Yep. That's almost exactly what I was experiencing with the flash messages. The first render, it was working great, and the second render no messages were displaying at all, because since flash messages are unset after they are displayed, there were no messages to display the second time through, causing me much confusion as to why they were not showing up. @ellotheth saved us both, man. :+1: :100:

ellotheth commented 10 years ago

Aw, shucks.