Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Support strict_vars globally in application config #76

Open
GeeH opened this issue Jun 28, 2016 · 1 comment
Open

Support strict_vars globally in application config #76

GeeH opened this issue Jun 28, 2016 · 1 comment

Comments

@GeeH
Copy link
Contributor

GeeH commented Jun 28, 2016

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html


Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/6127
User: @hschletz
Created On: 2014-04-12T18:08:09Z
Updated At: 2016-05-03T10:44:27Z
Body
I want a notice whenever a view template tries to access an undefined view variable via $this->var. I know only 2 ways to achieve this:

  1. Call $this->vars()->setStrictVars(true) in every template
  2. Set up an event listener and run setStrictVars(true) recursively on the view model and all its children (taking into account that a model's variables may be stored as an array instead of Zend\View\Variables).

Zend\View\Variables even supports an option "strict_vars" via its constructor, but neither this option nor setStrictVars() appear to be used anywhere within the ZF code.

It would be nice to be able to set strict_vars globally in the application configuration and have the Variables constructor evaluate it.


Comment

User: @Ocramius
Created On: 2014-04-13T07:34:32Z
Updated At: 2014-04-13T07:34:32Z
Body
@hschletz why not making it a small module that registers a listener, and that can be re-distributed and enabled/disabled?
Changing Variables#$strictVars can't really be done in 2.x as it looks like quite a BC break to me...


Comment

User: @hschletz
Created On: 2014-04-13T08:57:23Z
Updated At: 2014-04-13T08:57:23Z
Body
The change would not break BC if the default remains unchanged. I'm talking about a new option "strict_vars" in the application config. Since this option does not exist yet (only if passed manually to the Variables object), it will not be present in any existing application code - it would be an entirely new feature.

I think view variables should be treated like any other variable - ignoring reference to an undefined variable is bad practice.The hassle of an external module for this essential feature looks like overkill to me.


Comment

User: @Ocramius
Created On: 2014-04-13T08:59:28Z
Updated At: 2014-04-13T08:59:28Z
Body

I'm talking about a new option "strict_vars" in the application config. Since this option does not exist yet (only if passed manually to the Variables object), it will not be present in any existing application code - it would be an entirely new feature.

Yeah, for this path, I really suggest having a module that deals with it. It can be as minimal as EdpModuleLayouts


Comment

User: @hschletz
Created On: 2014-04-13T09:22:41Z
Updated At: 2014-04-13T09:22:41Z
Body
Sure, my workaround is not much code, but it's a bit quirky because it adds some postprocessing to all view models, instead of having things set up correctly in the first place. Additionally, I want to keep external dependencies as minimal as possible. This functionality really should be provided by the ZF core which currently not only encourages writing bad code, but makes writing better code needlessly complicated.

I'm not exaggerating with the "bad code" thing: Once enabled, the notices revealed several previously undiscovered bugs in one of my projects.


Comment

User: @Ocramius
Created On: 2014-04-13T16:37:30Z
Updated At: 2014-04-13T16:37:30Z
Body
I fully agree on the strictness here, it would indeed help, but I would really just change that default value in 3.x and have a debug module in 2.x


Comment

User: @Martin-P
Created On: 2014-04-15T16:04:54Z
Updated At: 2014-04-15T16:19:34Z
Body
Interesting info about setStrictVars().

I did some testing by attaching a listener to ViewEvent::EVENT_RENDERER and I am able to set strict vars on the ViewModel returned by the controller and on the layout. However, this does not cover the render() view helper and partial() view helper. I know this is not a forum for coding issues, but I think this can be useful for the actual implementation to make sure all parts are covered. Any ideas on how to cover that?

<?php

namespace MyStrictViewVars;

use Zend\EventManager\EventInterface;
use Zend\ModuleManager\Feature;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\View\Variables as ViewVariables;
use Zend\View\ViewEvent;

class Module implements Feature\BootstrapListenerInterface
{
    /**
     * @var ServiceLocatorInterface
     */
    protected $serviceManager;

    /**
     * Bootstrap event listener
     *
     * @param EventInterface $event
     */
    public function onBootstrap(EventInterface $event)
    {
        $application    = $event->getApplication();
        $eventManager   = $application->getEventManager();

        $identifier = 'Zend\View\View';
        $event      = ViewEvent::EVENT_RENDERER;
        $callback   = array($this, 'setStrictVars');
        $priority   = 100;
        $eventManager->getSharedManager()->attach($identifier, $event, $callback, $priority);

        $serviceManager = $application->getServiceManager();
        $this->setServiceManager($serviceManager);
    }

    /**
     * Set strict vars on ViewModel
     *
     * @param EventInterface $event
     */
    public function setStrictVars(EventInterface $event)
    {
        $config = $this->getServiceManager()->get('Config');
        if (!isset($config['view_variables']['strict_vars'])) {
            return;
        }

        $useStrictVars = (bool) $config['view_variables']['strict_vars'];
        $viewModel     = $event->getModel();

        /**
         * If $variables is not instance of ViewVariables,
         * convert it to ViewVariables and overwrite $viewModel variables
         */
        $variables = $viewModel->getVariables();
        if (!$variables instanceof ViewVariables) {
            $viewVariables = new ViewVariables($variables);
            $viewModel->setVariables($viewVariables, true);
        }

        $viewModel->getVariables()->setStrictVars($useStrictVars);
    }

    /**
     * @param ServiceLocatorInterface $serviceManager
     */
    protected function setServiceManager(ServiceLocatorInterface $serviceManager)
    {
        $this->serviceManager = $serviceManager;
    }

    /**
     * @return ServiceManager
     */
    protected function getServiceManager()
    {
        return $this->serviceManager;
    }
}

Comment

User: @Ocramius
Created On: 2014-04-15T17:12:55Z
Updated At: 2014-04-15T17:12:55Z
Body
@Martin-P the solution there would be to just replace the PhpRenderer IMO.


Comment

User: @Martin-P
Created On: 2014-04-16T22:13:43Z
Updated At: 2014-04-16T22:29:25Z
Body
Did some digging. The creation of PhpRenderer which is used in the application is hardcoded in the ViewManager on line 184 and the factory for the ViewManager is hardcoded in the ServiceListenerFactory on line 57. The ServiceListener is created even before the modules are loaded in the ModuleManagerFactory on line 44. It is possible to overwrite the ServiceListener via application.config.php:

return array(
    'service_manager' => array(
        'factories' => array(
            'ServiceListener' => 'ModuleName\Factory\ServiceListenerFactory',
        ),
    ),
    // more config
);

But now the namespace is still unknown, because the module is not yet loaded. Perhaps the dirty way by overriding the protected property $renderer of the ViewManager with Reflection? 😉

Edit: nevermind, this is becoming a mess 😃


Comment

User: @Martin-P
Created On: 2014-04-23T12:32:43Z
Updated At: 2014-04-23T12:32:43Z
Body
Always funny to read back your own comments 😄
Easy solution: indeed overwrite PhpRenderer and set HelperPluginManager and Resolver from the already existing PhpRenderer. Overwrite the methods get, __get and vars and set strict mode before they are called. Add an initializer to the view_helpers config and inject your custom PhpRenderer with the setView() method of the view helper.


Comment

User: @antiphp
Created On: 2016-04-11T10:41:35Z
Updated At: 2016-04-11T10:41:45Z
Body

why not making it a small module

Why not making it a small configuration option? :)

There is no reason to handle view variables different than regular variables, if they are not set but used.


Comment

User: @hschletz
Created On: 2016-05-02T17:24:39Z
Updated At: 2016-05-02T17:24:39Z
Body
@GeeH: Still relevant, don't close. All available/proposed solutions are far too complicated for such a basic code quality feature. @antiphp has it summed up pretty well. I'd also suggest making the desired behavior the default for ZF3.


Comment

User: @grizzm0
Created On: 2016-05-02T18:43:20Z
Updated At: 2016-05-02T18:43:20Z
Body
This sounds like the job of a module to me.


@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-view; a new issue has been opened at laminas/laminas-view#20.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants