New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diffrent volt partial() in php 5.6/7.0 #12176

Closed
FlexIDK opened this Issue Aug 29, 2016 · 22 comments

Comments

Projects
None yet
@FlexIDK

FlexIDK commented Aug 29, 2016

Expected and Actual Behavior

In PHP 5.6

Volt:
partial("email/tag_open", {'type': 'p'}) - $type is local var
{{ type }} - not set
partial("email/tag_open", {}) - $type inside - not set

In PHP 7

Volt:
partial("email/tag_open", {'type': 'p'}) - $type is global var
{{ type }} - last state set
partial("email/tag_open", {}) - $type inside - last state set

Details

  • Phalcon version: 3.0.1
  • PHP Version: 5.6.24-0+deb8u1 / 7.0.10-1~dotdeb+8.1
  • Operating System: Debian 8.5
  • Installation type: Compiling from source

Refs

@andresgutierrez

This comment has been minimized.

Member

andresgutierrez commented Aug 31, 2016

PHP 5 allowed us to create intermediate symbol tables per render, in php7 that is not possible, we have to research more about it.

@janisbiz

This comment has been minimized.

janisbiz commented Sep 25, 2016

I would like to suggest this fixing ASAP, as this is braking all the partial system in the views. (quite important thing)

This happens only on php7, as @andresgutierrez described, thought - php5 (with phalcon 3) is all good.

Also it is happening with .phtml, not only with Volt.

@challet

This comment has been minimized.

Contributor

challet commented Dec 22, 2016

It's also driving the loop context crazy :

{% for d in data %}
    {{ dump(loop.first) }} // true 
    // there is a {% for t in tags %} inside it
    {% include "header" with ['active': loop.first, 'tags': d.tags] %} 
    {{ dump(active) }} // true 
    {{ dump(loop.first) }} // false 
{% endfor %}
@jl91

This comment has been minimized.

jl91 commented Jan 26, 2017

some news about it? someone knows when it will be fixed? what branch ? or date? i've identified this problem in phalcon 3.0.0, but now i'm migrating to phalcon 3.0.3!

@Jurigag

This comment has been minimized.

Member

Jurigag commented Jan 26, 2017

Im guessing if someone would know then he would do PR with fix. We have to wait until @andresgutierrez will do something about this.

@jellisii

This comment has been minimized.

jellisii commented Jul 11, 2017

Can confirm this is still a problem, particularly the loop context that @challet referenced. I can put together a test case if needed, as this is causing me headaches.

@sergeyklay sergeyklay added the Volt label Oct 18, 2017

@janisbiz

This comment has been minimized.

janisbiz commented Nov 23, 2017

Can confirm as now. (23.11.2017). Due to this issue cannot upgrade two of my projects to php 7+ :( Same behavior is with .phtml, so @sergeyklay Volt label can be removed, as it's affecting all kind of views.

@sergeyklay sergeyklay removed the Volt label Nov 24, 2017

@sergeyklay sergeyklay self-assigned this Nov 24, 2017

@sergeyklay

This comment has been minimized.

Member

sergeyklay commented Nov 24, 2017

I'll try to sort out

@kgrammer

This comment has been minimized.

kgrammer commented Jan 25, 2018

Can we get a status update regarding this issue? Is it being addressed and if so, when might we expect to see the fix?

@stale

This comment has been minimized.

stale bot commented Jul 8, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Jul 8, 2018

@stale stale bot closed this Jul 9, 2018

@thnoack

This comment has been minimized.

thnoack commented Jul 9, 2018

With Phalcon 4 dropping PHP5 support, I believe this issue is becoming even more important. Is there any way one could possibly help to move this along?

@kgrammer

This comment has been minimized.

kgrammer commented Jul 9, 2018

Since this issue has not been fixed, I don't think it should have been closed by Mr. Stale Bot...

@sergeyklay sergeyklay reopened this Jul 9, 2018

@stale stale bot removed the stale label Jul 9, 2018

@zacek

This comment has been minimized.

zacek commented Jul 16, 2018

As the bug fixing can take some time, here is my workaround in the hope it helps someone: extend \Phalcon\Mvc\View\Engine\Volt and use it instead of the original class when creating voltService:

namespace My\Fix;

/*
 * This class is a workaround for a bug in zephir
 * Remove it when the bug is fixed, see https://github.com/phalcon/zephir/issues/1621
 */
class Volt extends \Phalcon\Mvc\View\Engine\Volt
{
    public function render($templatePath, $_params__, $_mustClean__ = false)
    {
        if ($_mustClean__) {
            ob_clean();
        }
        $compiler = $this->getCompiler();
        $compiler->compile($templatePath);
        $_compiledTemplatePath_ = $compiler->getCompiledTemplatePath();
        unset($compiler);
        unset($templatePath);

        if (is_array($_params__)) {
            foreach ($_params__ as $_key_ => $_value_) {
                $$_key_ = $_value_;
            }
        }
        require $_compiledTemplatePath_;
        if ($_mustClean__) {
            $this->_view->setContent(ob_get_contents());
        }
    }
}

The idea behind the fix is simple - I've just "copy-pasted" code from the zephir sources of the Volt class to PHP - in PHP it works as expected.

When the bux gets fixed simply replace \My\Fix\Volt with \Phalcon\Mvc\View\Engine\Volt and delete this class.

@stale

This comment has been minimized.

stale bot commented Oct 14, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Oct 14, 2018

@kgrammer

This comment has been minimized.

kgrammer commented Oct 14, 2018

As I commented when Mr. Stale Bit attempted to close this issue last time, since this issue has not been fixed, I don't think it should have been closed by Mr. Stale Bot...

And with that said, when might we expect this issue to be addressed?

@stale stale bot removed the stale label Oct 14, 2018

@niden

This comment has been minimized.

Member

niden commented Oct 15, 2018

Thank you @kgrammer

As for when, not sure. I have a few issues I need to check first for this week and I will check this out soon. I have to identify first where it comes from and whether we need to make changes to Zephir.

@yesworld

This comment has been minimized.

yesworld commented Oct 25, 2018

Unfortunately, @niden I have the same problem. #13551

@yesworld

This comment has been minimized.

yesworld commented Nov 13, 2018

@niden, any news?

@niden

This comment has been minimized.

Member

niden commented Nov 14, 2018

@yesworld Sorry I haven't had time to check this out. I will try but it won't be until the end of next week till I have some free time.

@harrywebster

This comment has been minimized.

harrywebster commented Nov 20, 2018

Thanks @zacek that's a great little work-around until this issue is resolved!

@Jurigag

This comment has been minimized.

Member

Jurigag commented Nov 20, 2018

Thanks to latest fixes to zephir I guess this will be soon fixed.

@sergeyklay sergeyklay added this to the 3.4.2 milestone Nov 20, 2018

sergeyklay added a commit that referenced this issue Nov 20, 2018

sergeyklay added a commit that referenced this issue Nov 20, 2018

@sergeyklay

This comment has been minimized.

Member

sergeyklay commented Nov 20, 2018

I hope we managed to sort out with this issue finally here #13606. All necessary changes are already in the appropriate branch. We will try to release the next version in the coming days. Thank you for your patience, the report, and for helping us make Phalcon better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment