phug-php / phug

Phug - The Pug Template Engine for PHP
https://phug.selfbuild.fr
MIT License
62 stars 3 forks source link

Page not update when change base template and enable cache #20

Closed Illvili closed 6 years ago

Illvili commented 6 years ago

Hello,

I encountered an issue with the following code: index.php:

use Pug\Pug;

include 'vendor/autoload.php';

$pug = new Pug([
    'basedir' => __DIR__ . '/templates',
    'cache_dir' => __DIR__ . '/cache'
]);

$pug->displayFile('child');

child.pug:

extends base
block content
    p in child

base.pug:

p in base
block content

at this time the final page is:

<p>in base</p><p>in child</p>

and change base.pug:

p in base updated!
block content

I expected to get:

<p>in base updated!</p><p>in child</p>

But I actually get:

<p>in base</p><p>in child</p>

the cached template code not update correctly when extends a base template

Thanks!

Illvili commented 6 years ago

and here is a list of composer show

js-phpize/js-phpize                     1.12.1
js-phpize/js-phpize-phug                1.1.7 
js-transformer/js-transformer           1.0.0
nodejs-php-fallback/nodejs-php-fallback 1.3.1 
phug/ast                                0.1.0
phug/compiler                           0.5.16
phug/dependency-injection               1.3.2
phug/event                              0.1.0
phug/formatter                          0.5.36
phug/js-transformer-filter              1.0.1 
phug/lexer                              0.5.18
phug/parser                             0.5.8
phug/phug                               0.2.2
phug/reader                             0.2.1
phug/renderer                           0.3.5
phug/util                               0.4.12
psr/http-message                        1.0.1 
pug-php/pug                             3.1.4 
symfony/polyfill-mbstring               v1.6.0
symfony/var-dumper                      v3.4.3
kylekatarnls commented 6 years ago

Hi,

We use last modifed time to test if a file is up to date and this is our unit test for that (https://github.com/phug-php/renderer/blob/master/tests/Phug/Adapter/FileAdapterTest.php#L266) passed successfully.

Can you check the following please: filemtime(your template) > filemtime(cache file) PS: (the option keep_base_name can help you to find the file in your cache directory as it will prepend "child" to the cache file) dump the imports.serialize.txt file of the cache file (it should contains base.pug)

Thanks

Illvili commented 6 years ago

stat -x templates/child.pug

  File: "templates/child.pug"
  Size: 42           FileType: Regular File
  Mode: (0644/-rw-r--r--)         Uid: (  501/ Illvili)  Gid: (   20/   staff)
Device: 1,4   Inode: 8596218700    Links: 1
Access: Mon Jan  8 14:41:06 2018
Modify: Mon Jan  8 14:30:41 2018
Change: Mon Jan  8 14:30:41 2018

stat -x cache/QZtu5-3MUHKYh8AzdqJ9RtbBd9vH89uu9XDoP9iN64k.php

  File: "cache/QZtu5-3MUHKYh8AzdqJ9RtbBd9vH89uu9XDoP9iN64k.php"
  Size: 518          FileType: Regular File
  Mode: (0644/-rw-r--r--)         Uid: (  501/ Illvili)  Gid: (   20/   staff)
Device: 1,4   Inode: 8596223268    Links: 1
Access: Mon Jan  8 14:41:08 2018
Modify: Mon Jan  8 14:41:03 2018
Change: Mon Jan  8 14:41:03 2018

cat cache/QZtu5-3MUHKYh8AzdqJ9RtbBd9vH89uu9XDoP9iN64k.php.imports.serialize.txt

a:0:{}%
kylekatarnls commented 6 years ago

OK, imports.serialize.txt should not be empty, it's the problem, it should contains base.pug reference. Something like this:

a:1:{i:0;s:123:"/absolute/path/base.pug";}

It's what I get when I execute your code.

When you update child.pug, does the imports.serialize.txt is modified (its contents or its modify last date)?

Illvili commented 6 years ago

imports.serialize.txt last modify date change, but its contents keeps a:0:{} when update child.pug

Illvili commented 6 years ago

I found something maybe useful: In file php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php function public function cache($path, $input, callable $rendered, &$success = null)

var_dump('before cache', $path, $this->getRenderer()->getCompiler()->getImportPaths());

$success = $this->cacheFile(
    $destination,
    $rendered($path, $input),
    $this->getRenderer()->getCompiler()->getImportPaths($path)
);

var_dump('after cache', $path, $this->getRenderer()->getCompiler()->getImportPaths());

add two var_dump, output:

/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:58:string 'before cache' (length=12)
/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:58:string 'child' (length=5)
/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:58:
array (size=0)
  empty
/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:66:string 'after cache' (length=11)
/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:66:string 'child' (length=5)
/php-jade/vendor/phug/renderer/src/Phug/Renderer/Adapter/FileAdapter.php:66:
array (size=1)
  '/php-jade/templates/child.pug' => 
    array (size=2)
      0 => string '/php-jade/templates/base.pug' (length=52)
      1 => string '/php-jade/templates/import_demo.pug' (length=59)
Illvili commented 6 years ago

php-jade/vendor/phug/compiler/src/Phug/Compiler.php function getImportPaths:

/**
 * @param string|null $path
 *
 * @return array
 */
public function getImportPaths($path = null)
{
    if ($path === null) {
        return $this->importPaths;
    }

    return isset($this->importPaths[$path])
        ? $this->importPaths[$path]
        : [];
}

when resolve $path using $path = $this->resolve($path); everything works fine, the imports.serialize.txt contain correct templates and update check also works fine

kylekatarnls commented 6 years ago

Great job, I could reproduce. Indeed it does not appears in our unit test as we first test the up-to-date case before the refresh case.

But with this order. 2 renders are needed to get the cache working:

$pug->renderFile('child');
$pug->displayFile('child');

Oviously, it's not a good solution. I will fix that.

kylekatarnls commented 6 years ago

It seems that in the code you found, the third argument is executed before the second. I really can't explain why PHP execute them in this order:

$success = $this->cacheFile(
    $destination,
    $rendered($path, $input),
    $this->getRenderer()->getCompiler()->getImportPaths($path)
);

A simple buffer var fix it:

$output = $rendered($path, $input);
$success = $this->cacheFile(
    $destination,
    $output,
    $this->getRenderer()->getCompiler()->getImportPaths($path)
);

I'm frustrated not to understand why. Tests passing then I will tag a version.

kylekatarnls commented 6 years ago

You can now use composer update to fix this bug. Feel free to re-open if you get trouble.

Illvili commented 6 years ago
$success = $this->cacheFile(
    $destination,
    $rendered($path, $input),
    $this->getRenderer()->getCompiler()->getImportPaths($path)
);

The code order is ok. But when getImportPaths the argument $path is child and the importPath is

array (size=1)
  '/php-jade/templates/child.pug' => 
    array (size=2)
      0 => string '/php-jade/templates/base.pug' (length=52)
      1 => string '/php-jade/templates/import_demo.pug' (length=59)

the path in compiler is store as full path but argument send to it not. Here is the function getImportPaths

/**
 * @param string|null $path
 *
 * @return array
 */
public function getImportPaths($path = null)
{
    if ($path === null) {
        return $this->importPaths;
    }

    return isset($this->importPaths[$path])
        ? $this->importPaths[$path]
        : [];
}

the $path should always resolve as full path like other Compiler member function

So, the real problem is

$pug->displayFile('child');

when change it to

$pug->displayFile('./templates/child.pug');

the imports.serialize.txt got correct contents

thanks!

@kylekatarnls

P.S. there is no reopen button.

kylekatarnls commented 6 years ago

Oh, indeed there are no more reopen button for people out of the project. I guess it was spammed in some projects.

Did you composer update to get the last renderer as it make pass your exact case: https://github.com/phug-php/renderer/blob/master/tests/Phug/Adapter/FileAdapterTest.php#L161

Illvili commented 6 years ago

In these lines: L188, L194, L206 the $child should be 'extends.pug' In my case, I didn't provide the base directory the Compiler's function getImportPaths cannot get correct imports

kylekatarnls commented 6 years ago

Thanks, so there were 2 problems, OK I reproduce it with relative paths. Will try to find something.

kylekatarnls commented 6 years ago

Hi, can you retry to composer update?

Illvili commented 6 years ago

It works! Thanks!

kylekatarnls commented 6 years ago

Not without pain, but I'm happy to hear that.