Warning with cakephp3.5

[Orken]

Hello,

When i run TwigView on a CakePhp 3.5, with this code,

{{ _view.assign('title', __("I'm title")) }}

{{ _view.start('header') }}
<p>I'm header</p>
{{ _view.end() }}

{{ _view.start('footer') }}
<p>I'm footer</p>
{{ _view.end() }}
<p>I'm content</p>

i got those warnings :

Warning (4096): Object of class App\View\AppView could not be converted to string [ROOT/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php, line 27]
I'm header

I'm title

Warning (4096): Object of class App\View\AppView could not be converted to string [ROOT/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php, line 22]
Warning (4096): Object of class App\View\AppView could not be converted to string [ROOT/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php, line 32]
Warning (4096): Object of class App\View\AppView could not be converted to string [ROOT/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php, line 42]
I'm content

Warning (4096): Object of class App\View\AppView could not be converted to string [ROOT/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php, line 37]
I'm footer

Everything works fine with Cake3.4

Mitch

[inoas]

Could you also include
/tmp/cache/twigView/93/9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.php

[Orken]

You'll find it attached here.

3d5e0211dc28c59bab413b8dac0ddbe4bc9521105d145d749578bb453c993008.zip

[Orken]

Sorry, I have included the wrong file, here's the good one.

9354f08c1f47e54248409d9f11b74ab4e9f9d332c0df7ef5d38a4a486cb660cb.zip

[robertpustulka]

Some methods return $this since 3.5.

I've found a workaround for this, use none filter:

{{ _view.assign('title', __("I'm title"))|none }}

[inoas]

What about

{% _view.assign('title', __("I'm title")) %} {# execute setter #} Edit: That's sadly not the way Twig works.
{{ _view.fetch('title') }} {# just echo #} Edit: fixed

[robertpustulka]

@inoas have you checked if your examples work, or maybe you are just guessing?

[inoas]

Half guessing. I do only use a few parts of TwigView in my projects and I am not using _view.*

Trying:
{% _view.assign('title', __("I'm title")) %} {# execute setter #} <= Not a valid tag
{{ _view.get('title') }} {# just echo #} <= not an existing property
{{ _view.title }} {# this may work as well, try-and-see #} <= not an existing property

p.s.: I could not even find the docs for none https://twig.symfony.com/doc/2.x/filters/index.html

[robertpustulka]

Ok, so please don't mislead people :)

none filter is provided by this plugin.

[inoas]

Ok, so please don't suggest to use dirty hacks :)

The problem here is that CakePHPs minor release is non-BC.

[robertpustulka]

As I said it is a workaround which actually works.

[inoas]

It does not.

You still need {{ _view.fetch('title') }} as |none will stop returning $this but not output the title as before as CakePHP 3.5 broke that feature non-BC.

[robertpustulka]

I'll ask again: have you tried this or still guessing?

[inoas]

Have you, I have.

[robertpustulka]

I use this workaround successfilly in my code.

Are we talking about the same?

{{ _view.assign('block', 'value')|none }}
{{ _view.fetch('block') }}

[inoas]

Yes that will work, despite being a hack (running echo return, looking at WyriHaximus\TwigView\Lib\Twig\Extension\Strings). Without the fetch call it won't work.

Can you see a better way to get around the hack required because of assign/set returning $this in CakePHP 3.5? The only way I found is creating quite a bunch of Twig-Tags as far as I understand: https://twig.symfony.com/doc/2.x/advanced.html#tags as {% foo %} seems only to allow DSL constructs and no Twig Simple Functions.

[markstory]

We should probably revert the changes in CakePHP then.

[inoas]

Well 3.5 has been out for some time now and that could have impacts too(?) so my suggestion (disclaimer @robertpustulka I have tried it, right now ;-) would be, to overwrite assign() and __set() in class TwigView to not return anything?

// in TwigView:
    public function assign($name, $value)
    {
        $this->Blocks->set($name, $value);
    }

[markstory]

@inoas That is another option as well. I can put together a pull request for that if it would be helpful.

[inoas]

I'll wait for @WyriHaximus to chime in and if he concurs I will do that and hopefully also add working unit tests.

[robertpustulka]

@inoas This could work 👍

[inoas]

Summary: We should either revert CakePHP 3.5 or fix TwigView 4.x & 5.x to make sure existing Twig templates run without touching them (production etc).

A quick copy&adapt. I am not sure we ll need/want all of them (consistency? would you set the template path within a twig-template, ever, etc)?

In class TwigView:

    /**
     * {@inheritDoc}
     * @return void
     */
    public function setTemplatePath($path)
    {
        parent::setTemplatePath($path);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function setLayoutPath($path)
    {
        parent::setLayoutPath($path);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function enableAutoLayout($enable = true)
    {
        parent::enableAutoLayout($enable);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function setTemplate($name)
    {
        parent::setTemplate($name);
    }

These for sure:

    /**
     * {@inheritDoc}
     * @return void
     */
    public function setTheme($theme)
    {
        parent::setTheme($theme);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function setLayout($name)
    {
        parent::setLayout($name);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function start($name)
    {
        parent::start($name);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function prepend($name, $value)
    {
        parent::prepend($name, $value);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function append($name, $value)
    {
        parent::append($name, $value);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function assign($name, $value)
    {
        parent::assign($name, $value);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function reset($name)
    {
        parent::reset($name);
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function end()
    {
        parent::end();
    }

    /**
     * {@inheritDoc}
     * @return void
     */
    public function extend($name)
    {
        parent::extend($name);
    }

Question: I assume about the same amount of functions that should go inside TwigView would otherwise need to become Twig Tags for TwigView + CakePHP 3.5 to run without changes on Templates?

[markstory]

Another option I thought of is to implement __toString() on View, and return ''. This would solve the compatibility issue for both this and other plugins.

[inoas]

I am going to give this a try before hacking other ways.

[inoas]

I gave it a try locally (in my SimpleTwigView ... but it should be the same).
Following works without being required to change any twig template code:

Within your class TwigView

    /**
     * {@inheritDoc}
     * @return string
     */
    public function assign($name, $value)
    {
        parent::assign($name, $value);

        return $value;
    }

    /**
     * {@inheritDoc}
     * @return string
     */
    public function __toString()
    {
        return '';
    }

Tested with

<h1>{{ _view.assign('title', __("I'm title")) }}</h1>
{{ _view.start('header') }}
<p>I'm header</p>
{{ _view.end() }}
{{ _view.start('footer') }}
<p>I'm footer</p>
{{ _view.end() }}
<p>I'm content</p>
{{ _view.fetch('header')|raw }}
{{ _view.fetch('footer')|raw }}

Output

<h1>I'm title</h1>
<p>I'm content</p>
<p>I'm header</p>
<p>I'm footer</p>

@WyriHaximus is this approach good with you?

Would any of you want to have any further setter methods to return their values for BC reasons?

[WyriHaximus]

@inoas I'll test it and dive into this issue tomorrow 👍

[markstory]

@inoas I was proposing to implement __toString() on CakePHP's view class.

[dereuromark]

The problem here is that CakePHPs minor release is non-BC.

@inoas You are dead-wrong. This is not a BC break from the framework perspective.
"Void to sth" is always fully BC.
If the plugin and Twig used the return value of void methods and by accident just worked so far, that is the plugin's problem, not CakePHP core's. Please understand this difference.

That said, the solution now provided seems to fix this in some way. So seems to be all good with the next patch release. Just be a bit more careful with your accusations, please :)

[robertpustulka]

Ideally a "native" support for Cake blocks would be great.

Something like:

{% start 'header' %}
<h1>Title</h1>
{% end %}

{% append 'header' %}
<h2>Subtitle</h2>
{% end %}

{% fetch 'header' %}

[inoas]

@markstory I am not sure that that is entirely good "implement __toString() on CakePHP's view class":

<h1>{{ _view.assign('title', __("I'm title")) }}</h1> would run without any warnings, but render an empty <h1/> and in less of a chance for the app-developer to catch the problem.

@dereuromark not sure if I am the one who is dead wrong, this time ;-):

1. If the change was from any-non-void to void; How would that be BC? Say a transformator suddenly storing the transformed value instead of returning it?

2. The change here is from returning string to $this which is a return type change and that is non-BC, can you confirm this?

Edit: assign() never returned something despite that the TwigView docs made me believe it would.

@robertpustulka yep, however:

1. do you agree that we should not force people to upgrade their Twig Templates during a minor CakePHP release, let alone they know about the problem. If we find a rather good compromise, It is bad enough that templates will break unless people will update/upgrade to the upcoming 4.x TwigView as well. To do that the solution above is fitting, or can you think about anything better?

2. After (1.) we can prep a PR to add Twig-Tags for start, append, prepend, assign, fetch, reset, and end and deprecate the old way to use them?

3. In a future TwigView release we can remove the old way of calling {{ _view.some_setter() }}?

[robertpustulka]

The change here is from returning string to $this which is a return type change and that is non-BC, can you confirm this?

The change is from returning void to $this (see the code).

I suspect that you expected that assign() would have returned a block content. The behavior has not changed, only the return value has - from nothing to something.

Please, again, get your facts straight before you start telling people that they are wrong.

do you agree that we should not force people to upgrade their Twig Templates during a minor CakePHP release

Yes. I'm talking about a future plugin release.

IMO there needs to be some other way of manipulaitng Cake blocks. Currently the state changing functions (start(), assign(), append(), etc) are wrapped in echo blocks which isn't the good idea.

[inoas]

As assign() never returned something despite that the TwigView docs made me believe it would, implementing __toString() in CakePHP 3.5 like suggested will fix this problem.

I can take care and implement the tags for start, append, prepend, assign, fetch, reset, end (tags do not (necessarily) echo) on top of the next major TwigView release.

[dereuromark]

@inoas But you complained about 3.5 in particular. But the only change there was cakephp/cakephp#11134 which is BC (void to $this is always BC as per BC guidelines).
Any other change regarding assign() seems to be in a very early 3.x version, though. So quite some years ago.

[inoas]

@Orken #61 will fix your problem once merged and tagged.

#62 is the followup issue to track the correct implementation in future, for all those who are interested.

[WyriHaximus]

4.0.6 has been tagged with a fix for this thanks to @inoas 👍

[inoas]

Same/Similar Issue: cakephp/cakephp#11276 (comment)