Talesoft / tale-jade

A complete and fully-functional implementation of the Jade template language for PHP
http://jade.talesoft.codes
MIT License
88 stars 10 forks source link

check template's filemtime() before taking compiled from cache #127

Open cronfy opened 7 years ago

cronfy commented 7 years ago

When ttl option is set, tale-jade does not care whether source template was changed or not. It just sends cached file to render.

It would be desirable to have an option to check source tempate filemtime() previously. And if source template was changed, regenerate the cache for it.

E. g. 'auto_update' => true|false.

kylekatarnls commented 7 years ago

Just curious but why this behaviour? IMHO, ttl expected behaviour should be the same than any ttl cache option (dns for example). For ttl = 5 minutes, it means if something change, you will have to wait 5 minutes until the change will be visible for everyone. If you want instant re-compile on change, what would the ttl for?

cronfy commented 7 years ago

The reason is simple: when I deploy new version of app to production, I do not want to clear all Jade cache, I want to update only templates that changed.

Well, if this feature is imlemented, ttl probably would not be required.

kylekatarnls commented 7 years ago

Tale-jade use file_exists so just remove all .pug file in your cache directory on deploy and it should works:

rm cache/views/**/*.pug
cronfy commented 7 years ago

Tale-jade use file_exists so just remove all .pug file in your cache

This is the exact thing I want to avoid.

kylekatarnls commented 7 years ago

Why? Clear cache is a very common deployment operation.

cronfy commented 7 years ago

Because new version deployed != cache clear required. We deploy very often, if we would clear cache every time, caching itself would be ineffective.

TorbenKoehn commented 7 years ago

The File renderer already uses filemtime for the checks, see this line

If you like a different functionality like, I don't know, storing MD5/CRC-hashes in a JSON file to check against or something, I suggest you overwrite Tale\Jade\Renderer\Adapter\File or write an own renderer adapter by extending Tale\Jade\Renderer\AdapterBase.

After you did that, you can just pass that adapter to Tale\Jade\Renderer


use Tale\Jade;

$renderer = new Jade\Renderer([
    'adapter' => YourCustomFileAdapter::class,
    'adapter_options' => [
        //...your custom adapter options
    ]
]);

Take a look at the File-adapter to check how it works (or copy it and modify it accordingly)

If you require any help, ask :)

cronfy commented 7 years ago

No-no-no, I do not want MD5 etc! Just mtime check.

But the line you point to checks mtime for cached file. I speak about checking mtime of source file, like this:

$compileRequired = false;
if (!file_exists($outputPath)) {
    $compileRequired = true;
} else {
    $sourceMtime = filemtime($path);
    $cachedMtime = filemtime($outputPath);
    if ($sourceMtime > $cachedMtime || time() - $cachedMtime >= $this->getOption('ttl')) {
            $compileRequired = true;
    }
}
 if ($compileRequired) {
    ...
    file_put_contents($outputPath, $this->getRenderer()->compileFile($path));
}
TorbenKoehn commented 7 years ago

That's not easy to do, as the renderer doesn't know the exact location of the Jade file, only the Compiler knows. This will, however, be possible in Phug as that one will provide actual path-resolvers.

Would it be okay for you waiting for Phug? You can continue to use Tale Jade freely, there will be an extension for Phug that mimics Tale Jade's features completely.

cronfy commented 7 years ago

Of course, thanks. Can't wait to see Phug in action!