pug-php / pug

Pug template engine for PHP
https://www.phug-lang.com
MIT License
391 stars 42 forks source link

I could not call method of object as template local variable #200

Closed Freest10 closed 6 years ago

Freest10 commented 6 years ago

Hello,

I encountered an issue with the following code: //php file

   $pug = new Pug(array(
        'cache' => $cachePath,
        'pugjs' => true,
        'expressionLanguage' => 'js',
        'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
    ));

   $macroses_create = new Macroses();

   // class Macroses has getPagesById method

    $customVariables = array(
        'page_info' => $page_info_create,
        'macroses' => $macroses_create,
        'custom_macroses' => $custom_macroses_create,
        'req_params' => $page_info->getParams(),
        'req_method' => $page_info->getRequestMethod()
    );
    echo $pug->renderFileWithPhp($pathToIndexPugTemplate,  customVariables);

//pug file

- var getGlobalPageParams = {pageIds:[2],noActive:true,groups:["menu_group"]};
- var getGlobalPage = macroses.getPagesById(getGlobalPageParams);

I expected to get: the php method will be called

But I actually get: it was error

I try different settings, but it does not help. In previous pug-php(2) version it worked

Thanks!

kylekatarnls commented 6 years ago

Yes, this is explained here in the README: https://github.com/pug-php/pug#templates-from-pug-js and here in the documentation:

The pugjs use the native node package and PHP pass all the data flatten as JSON to it, so there is no way execute any PHP function inside the template. You have to choose between the JS engine or the PHP one.

Without viewing your templates it's hard to say but I think you do not need the pugjs option and can use the PHP engine. It seems more appropriate to stuff like macroses.getPagesById(getGlobalPageParams)

Freest10 commented 6 years ago

I created a cms based on the pug-php (soon release), which works equally for nodejs and php. The template should be on js and work equally with php and nodejs platforms. Then I lose backward compatibility with the nodejs platform, this is the main counter of pug-php. Can you return this feature to pug-php, please?

kylekatarnls commented 6 years ago

You still can execute $macroses->getPagesById($getGlobalPageParams) in your controller before passing the data to the view, so you can keep using the pugjs option.

Else the expressionLanguage option is here to convert JS-style expression to PHP (when it's possible since there is no PHP equivalent for all possible stuff in JS). And when saying pugjs allow a better compatibility, we means it will work exactly the same for a given template with a given input; but in your case the macroses object you potentially passed in a nodejs environment is not strictly the same than the $macroses you created in PHP. Make it work the same in your template would literally mean we should embed a engine to re-write your PHP object in JS by reflexion at each render. Sorry, not enough spare time for that and it really sounds like performance-killer.

Alternatives exist and are documented. The lonely way to get PHP data into JS execution without flatten the data is V8js. And running both JS and PHP in parallel is not that simple, that's not the pug-php purpose and not something planned for the near future (I don't understand neither the "return this feature", as it has never been possible in pug-php and, as far as I know, in any other Pug PHP port).

Freest10 commented 6 years ago

"You still can execute $macroses->getPagesById($getGlobalPageParams) in your controller before passing the data to the view, so you can keep using the pugjs option."

Yes, I can, but I must have a problem with polymorphism. Method "getPagesById" must be in different classes.

"Make it work the same in your template would literally mean we should embed a engine to re-write your PHP object in JS by reflexion at each render"

In previous version it was and it was cool.

"I don't understand neither the "return this feature", as it has never been possible in pug-php and, as far as I know, in any other Pug PHP port"

I introduced the pug-php into my project about 1.5 years ago, I can not say the version. The above feature works there, today decided to upgrade and it crashed.

Freest10 commented 6 years ago

I have archived the sources pug-php into my project. pug.zip

kylekatarnls commented 6 years ago

"In previous version it was and it was cool."

No, I can ensure you it never did. - var getGlobalPage = macroses.getPagesById(getGlobalPageParams); still works with just the option 'expressionLanguage' => 'js' but it does not run nodejs under the hood (not like the pugjs do).

All versions are still downloadable here: https://github.com/pug-php/pug/releases

1.5 year ago, the compatibility with JS syntax was very less advanced. The pugjs option did not exist so it was not possible at all to run the nodejs package. And js-phpize was not enabled in every places in templates. Please specify the code you was able to render before you no longer can with just the option 'expressionLanguage' => 'js'.

Freest10 commented 6 years ago

"No, I can ensure you it never did. - var getGlobalPage = macroses.getPagesById(getGlobalPageParams); still works with just the option 'expressionLanguage' => 'js' but it does not run nodejs under the hood (not like the pugjs do)."

Yes it work with this option. I dont need nodejs under hood, just pug syntax which i know must work on nodejs pug library.

"1.5 year ago, the compatibility with JS syntax was very less advanced" Totaly agree)

kylekatarnls commented 6 years ago

Great. Do you confirm expressionLanguage solves your problem?

Freest10 commented 6 years ago

No, I need use js methods in template and call php methods as global variable in pug template. This option did not help.

$pug = new Pug(array(
            'cache' => $cachePath,
            'pugjs' => true,
            'expressionLanguage' => 'js',
            'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
        ));

    $customVariables = array(
            'page_info' => $page_info_create,
            'testFn' => function(){
                return 'FFFdd';
            }
        );
        echo $pug->renderFileWithPhp($pathToIndexPugTemplate, customVariables);

pug file

h1 Hello from Scale!
- var h = testFn();

It works

if i try use js methods in pug file like this

h1 Hello from Scale!
- var t = 'test';
!= t.indexOf('t')
- var h = testFn();

it must be error

kylekatarnls commented 6 years ago

indexOf is supported by js-phpize. Please remove the pugjs option and give me an actual failure you get.

In most case, this is something you can remove from your template (put in in the controller for example). And if you use a pattern such as MVC, having as few treatment in the template as possible is also a best practice.

Then the approach is:

And it should not be needed with a clean view-controller organization but the only way to run JS code with PHP objects is http://php.net/manual/fr/class.v8js.php, sadly this is a PECL extension, it means a manual installation needed. So it would not be possible to put this directly in pug-php.

Freest10 commented 6 years ago

I removed pugjs option pug-php-error

kylekatarnls commented 6 years ago

The same exact code works on the demo: http://pug-demo.herokuapp.com/

Did you keep 'expressionLanguage' => 'js',?

Freest10 commented 6 years ago

I found an error - cacheDirectory method. It need me in old version of pug-php. I removed it and it worked. $pug->cacheDirectory(CURRENT_WORKING_DIR . '/client/templates/pug');

There was one problem - calling class methods in a template In the old version it worked. pug-php-old

pug file pug-file-old I understand that you need to call static methods, as you wrote above, but I really need instances of classes in the template, please)

kylekatarnls commented 6 years ago

I'm not a magican, I explained every possible usages. Here is an example of V8Js use:

<?php

include_once __DIR__.'/vendor/autoload.php';

function renderPug($template, $locals = array()) {
    $save = array();
    $keys = array();

    foreach ($locals as $key => $value) {
        $keys[$key] = $key;
        $save[$key] = isset($GLOBALS[$key]) ? $GLOBALS[$key] : null;
        $GLOBALS[$key] = $value;
    }

    $v8js = new V8Js('self', $keys);

    $pug = new \Phug\Renderer(array(
        'patterns' => [
            'transform_expression' => function ($jsCode) {
                return '$v8js->executeString('.var_export($jsCode, true).')';
            },
        ],
    ));

    $html = $pug->render($template, array(
        'v8js' => $v8js,
    ));

    foreach ($save as $key => $value) {
        $GLOBALS[$key] = $value;
    }

    return $html;
}

$d = new DateTime('yesterday');

echo renderPug('p=self.date.format("F")', array(
    'date' => $d,
));

I guess cacheDirectory does not work as you include files manually (some should miss). You really shouldn't include third-party with include, use composer. Every PHP libraries and framework are based on composer today.

Freest10 commented 6 years ago

Fixed connection, but it did not help. Without cacheDirectory it works.


use Macros\CustomMacros as CustomMacros;
use Macros\Macros as Macros;
use PageInfo\PageInfo as PageInfo;
use Pug\Pug;

include_once CURRENT_WORKING_DIR . '/vendor/autoload.php';

class ClientTemplate
{

    public function renderClient()
    {
        include_once CURRENT_WORKING_DIR . '/client/custom_macros/index.php';
        include_once CURRENT_WORKING_DIR . '/client/macros/index.php';
        include_once CURRENT_WORKING_DIR . '/client/page_info/index.php';
        include_once CURRENT_WORKING_DIR . '/client/macros/PugAdapter.php';
        include_once CURRENT_WORKING_DIR . '/client/custom_macros/PugAdapter.php';

        $cachePath = CURRENT_WORKING_DIR . '/client/templates/cache';
        $pathToIndexPugTemplate = CURRENT_WORKING_DIR . '/client/templates/pug/index.pug';
        $pug = new Pug(array(
            'cache' => $cachePath,
            'expressionLanguage' => 'js',
            'upToDateCheck' => false,
            'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
        ));

       list($success, $errors) = $pug->cacheDirectory(CURRENT_WORKING_DIR . '/client/templates/pug');

        $macros = new Macros();
        $customMacros = new CustomMacros();
        $macrosPugAdapter = new \Macros\PugAdapter($macros);
        $customMacrosPugAdapter = new \CustomMacros\PugAdapter($customMacros);

        $page_info = new PageInfo();
        $page_info_create = $page_info->getPageInfo();
        $customVariables = array(
            'pageInfo' => $page_info_create,
            'reqParams' => $page_info->getParams(),
            'reqMethod' => $page_info->getRequestMethod()
        );
         echo $pug->renderFileWithPhp($pathToIndexPugTemplate,
            array_merge($customVariables, $macrosPugAdapter->getMacros(), $customMacrosPugAdapter -> getCustomMacros()), null);
    }

    private function isProduction()
    {
        $config = MainConfiguration::getInstance();
        return $config->get("system", "production_mode");
    }
}
kylekatarnls commented 6 years ago

You should not run cacheDirectory at each render. The cacheDirectory should be ran when you deploy your app in production (and if templates have been updated). Then methods like renderFileWithPhp will use this cache.

So to properly understand: by default upToDateCheck is true, and so cacheDirectory is not needed, Pug will check if cache exist or not before to render and it will cache the compiled templates the first time you render it. This way cache is enabled by just passing a directory to the cache option. And in most situations it's good enough.

If you want to very optimize the cache, you can run cacheDirectory in an other script you call with CLI, an admin page or a deploy script. Then you can se upToDateCheck to false, and it will assume the cache files are already there without check.

Then when you say Without cacheDirectory it works. I also need to know what error you get when you use it else I cannot figure it out.

Freest10 commented 6 years ago

I changed the code

<?php

use Macros\CustomMacros as CustomMacros;
use Macros\Macros as Macros;
use PageInfo\PageInfo as PageInfo;
use Pug\Pug;

include_once CURRENT_WORKING_DIR . '/vendor/autoload.php';

class ClientTemplate
{

    public function renderClient()
    {
        include_once CURRENT_WORKING_DIR . '/client/custom_macros/index.php';
        include_once CURRENT_WORKING_DIR . '/client/macros/index.php';
        include_once CURRENT_WORKING_DIR . '/client/page_info/index.php';
        include_once CURRENT_WORKING_DIR . '/client/macros/PugAdapter.php';
        include_once CURRENT_WORKING_DIR . '/client/custom_macros/PugAdapter.php';

        $cachePath = CURRENT_WORKING_DIR . '/client/templates/cache';
        $pathToIndexPugTemplate = CURRENT_WORKING_DIR . '/client/templates/pug/index.pug';
        $pug = new Pug(array(
            'cache' => $cachePath,
            'expressionLanguage' => 'js',
            'upToDateCheck' => !$this->isProduction(),
            'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
        ));

        if ($this->isProduction()) {
            list($success, $errors) = $pug->cacheDirectory(CURRENT_WORKING_DIR . '/client/templates/pug');
        }

        $macros = new Macros();
        $customMacros = new CustomMacros();
        $macrosPugAdapter = new \Macros\PugAdapter($macros);
        $customMacrosPugAdapter = new \CustomMacros\PugAdapter($customMacros);

        $page_info = new PageInfo();
        $page_info_create = $page_info->getPageInfo();
        $customVariables = array(
            'pageInfo' => $page_info_create,
            'reqParams' => $page_info->getParams(),
            'reqMethod' => $page_info->getRequestMethod()
        );
         echo $pug->renderFileWithPhp($pathToIndexPugTemplate,
            array_merge($customVariables, $macrosPugAdapter->getMacros(), $customMacrosPugAdapter -> getCustomMacros()),);
    }

    private function isProduction()
    {
        $config = MainConfiguration::getInstance();
        return !!$config->get("system", "production_mode");
    }
}

When production mode it show error

kylekatarnls commented 6 years ago

First, the problem is still the same, in production, you will run the cacheDirectory at each render, so instead of optimizing your cache, it will re-render the whole directory for each user request. Then, in both case you should have a cache file generated in your cache directory, can you check if there are different and if so give them here. Thanks.

Freest10 commented 6 years ago

If I understood correctly, then it should look like this. 'cacheDirectory' call once in my code.

<?php

use Macros\CustomMacros as CustomMacros;
use Macros\Macros as Macros;
use PageInfo\PageInfo as PageInfo;
use Pug\Pug;

include_once CURRENT_WORKING_DIR . '/vendor/autoload.php';
include_once CURRENT_WORKING_DIR . '/client/custom_macros/index.php';
include_once CURRENT_WORKING_DIR . '/client/macros/index.php';
include_once CURRENT_WORKING_DIR . '/client/page_info/index.php';
include_once CURRENT_WORKING_DIR . '/client/macros/PugAdapter.php';
include_once CURRENT_WORKING_DIR . '/client/custom_macros/PugAdapter.php';

class ClientTemplate
{
    private $config;
    function __construct()
    {
        $this->config = MainConfiguration::getInstance();
    }

    public function renderClient()
    {
        $cachePath = CURRENT_WORKING_DIR . '/client/templates/cache';
        $pathToIndexPugTemplate = CURRENT_WORKING_DIR . '/client/templates/pug/index.pug';

        if ($this->isProduction()) {
            if ($this->isDeployed()) {
                $pug = new Pug(array(
                    'cache' => $cachePath,
                    'expressionLanguage' => 'js',
                    'upToDateCheck' => false,
                    'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
                ));

                echo $pug->renderFileWithPhp($pathToIndexPugTemplate, $this->getGlobalVariablesToPug());
            } else {
                $pug = new Pug(array(
                    'cache' => $cachePath,
                    'expressionLanguage' => 'js',
                    'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
                ));

                list($success, $errors) = $pug->cacheDirectory(CURRENT_WORKING_DIR . '/client/templates/pug');
                $this->config->set("cache_pug", "main", "1");
            }
        } else {
            $pug = new Pug(array(
                'cache' => $cachePath,
                'expressionLanguage' => 'js',
                'basedir' => CURRENT_WORKING_DIR . '/client/templates/pug',
            ));
            $this->config->set("cache_pug", "main", "0");

            echo $pug->renderFileWithPhp($pathToIndexPugTemplate, $this->getGlobalVariablesToPug());
        }
    }

    private function isProduction()
    {
        return !!$this->config->get("system", "production_mode");
    }

    private function isDeployed()
    {
        return !!$this->config->get("cache_pug", "main");
    }

    private function getGlobalVariablesToPug()
    {
        $macros = new Macros();
        $customMacros = new CustomMacros();
        $macrosPugAdapter = new \Macros\PugAdapter($macros);
        $customMacrosPugAdapter = new \CustomMacros\PugAdapter($customMacros);
        $page_info = new PageInfo();
        $page_info_create = $page_info->getPageInfo();
        $customVariables = array(
            'pageInfo' => $page_info_create,
            'reqParams' => $page_info->getParams(),
            'reqMethod' => $page_info->getRequestMethod()
        );

        return array_merge($customVariables, $macrosPugAdapter->getMacros(), $customMacrosPugAdapter->getCustomMacros());
    }
}

Error after cached error

What I did not correctly?)

kylekatarnls commented 6 years ago

Yes, if config->set is persistent, this is the right approach, then on deploy you should set to 0 to invalidate it and refresh. But to avoid the first user to have to wait the whole rendering, you can do that in an other file, URL or via a command line process. In fact, you're not supposed to run cacheDirectory then renderFileWithPhp just after. Even if it's theoretically possible, it's supposed to be done in seperated processes.

Else, can your provide the cache file for inspection: templates\cache\FRItO...?

Then can you confirm you do not get the error the first time, then get it every other times on refresh?

Freest10 commented 6 years ago

Yes, everything is right 'cacheDirectory' is called on the first request, all other 'renderFileWithPhp'. Cache file (production), which show an error.

<?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(7);
// PUG_DEBUG:7
 ?><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(1);
// PUG_DEBUG:1
 ?><h1><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(0);
// PUG_DEBUG:0
 ?>Hello from Scale!</h1><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(2);
// PUG_DEBUG:2
 ?><?php $t = array( 'ffgt' ) ?><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(3);
// PUG_DEBUG:3
 ?><?php $GLOBALS['__jpv_dotWithArrayPrototype_with_ref']($t, 'push')('dd') ?><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(4);
// PUG_DEBUG:4
 ?><?php $g = array( 'p' => 10 ) ?><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(5);
// PUG_DEBUG:5
 ?><?php $g = $GLOBALS['__jpv_set_with_ref']($g, 'f', '=', 4230) ?><?php 
\Phug\Renderer\Profiler\ProfilerModule::recordProfilerDisplayEvent(6);
// PUG_DEBUG:6
 ?><?= (is_bool($_pug_temp = $GLOBALS['__jpv_dotWithArrayPrototype_with_ref']($g, 'f')) ? var_export($_pug_temp, true) : $_pug_temp) ?>

When I turn off the production mode it success render.

kylekatarnls commented 6 years ago

The declaration of __jpv_dotWithArrayPrototype_with_ref is missing, this seems to be a regression from js-phpize version 2.0.0.

Thanks for your help, now we found the root cause, I will be able to patch it quickly.

kylekatarnls commented 6 years ago

I will tag a release with the fix as soon as checks end.

kylekatarnls commented 6 years ago

Now it should work after running composer update

Freest10 commented 6 years ago

Thank you)