Skip to content

Commit

Permalink
Load helpers at View construction.
Browse files Browse the repository at this point in the history
Loading helpers earlier in View's lifecycle allows for the removal of
many duplicated code segments and a now useless property. It slightly
modifies how View behaves in a test case, but that issue is easily
remedied by calling loadHelpers() a second time.

This primarily fixes issues where helpers may not be loaded in View
subclasses if they override any of View's methods. This is particularly
problematic when aliased helpers are involved.

Refs #4030
  • Loading branch information
markstory committed Aug 28, 2013
1 parent e7672b9 commit b8320fd
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 20 deletions.
3 changes: 2 additions & 1 deletion lib/Cake/Test/Case/Utility/DebuggerTest.php
Expand Up @@ -334,6 +334,8 @@ public function testExportVar() {
response => object(CakeResponse) {}
elementCache => 'default'
elementCacheSettings => array()
Html => object(HtmlHelper) {}
Form => object(FormHelper) {}
int => (int) 2
float => (float) 1.333
Expand All @@ -358,7 +360,6 @@ public function testExportVar() {
)
[protected] _scripts => array()
[protected] _paths => array()
[protected] _helpersLoaded => false
[protected] _parents => array()
[protected] _current => null
[protected] _currentType => ''
Expand Down
2 changes: 1 addition & 1 deletion lib/Cake/Test/Case/View/HelperTest.php
Expand Up @@ -975,7 +975,7 @@ public function testThatHelperHelpersAreNotAttached() {
$Helper->OtherHelper;

$result = $this->View->Helpers->enabled();
$expected = array();
$expected = array('Html');
$this->assertEquals($expected, $result, 'Helper helpers were attached to the collection.');
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Cake/Test/Case/View/JsonViewTest.php
Expand Up @@ -146,8 +146,8 @@ public function testRenderWithView() {
)
);
$Controller->set('user', $data);
$Controller->helpers = array('Paginator');
$View = new JsonView($Controller);
$View->helpers = array('Paginator');
$output = $View->render('index');

$expected = array('user' => 'fake', 'list' => array('item1', 'item2'), 'paging' => array('page' => 2));
Expand Down
18 changes: 1 addition & 17 deletions lib/Cake/View/View.php
Expand Up @@ -253,13 +253,6 @@ class View extends Object {
*/
protected $_paths = array();

/**
* Indicate that helpers have been loaded.
*
* @var boolean
*/
protected $_helpersLoaded = false;

/**
* The names of views and their parents used with View::extend();
*
Expand Down Expand Up @@ -347,6 +340,7 @@ public function __construct(Controller $controller = null) {
}
$this->Helpers = new HelperCollection($this);
$this->Blocks = new ViewBlock();
$this->loadHelpers();
parent::__construct();
}

Expand Down Expand Up @@ -460,9 +454,6 @@ public function render($view = null, $layout = null) {
if ($this->hasRendered) {
return true;
}
if (!$this->_helpersLoaded) {
$this->loadHelpers();
}
$this->Blocks->set('content', '');

if ($view !== false && $viewFileName = $this->_getViewFileName($view)) {
Expand Down Expand Up @@ -511,9 +502,6 @@ public function renderLayout($content, $layout = null) {
return $this->Blocks->get('content');
}

if (!$this->_helpersLoaded) {
$this->loadHelpers();
}
if (empty($content)) {
$content = $this->Blocks->get('content');
}
Expand Down Expand Up @@ -881,7 +869,6 @@ public function loadHelpers() {
list(, $class) = pluginSplit($properties['class']);
$this->{$class} = $this->Helpers->load($properties['class'], $properties['settings']);
}
$this->_helpersLoaded = true;
}

/**
Expand Down Expand Up @@ -1194,9 +1181,6 @@ protected function _elementCache($name, $data, $options) {
* @return string
*/
protected function _renderElement($file, $data, $options) {
if (!$this->_helpersLoaded) {
$this->loadHelpers();
}
if ($options['callbacks']) {
$this->getEventManager()->dispatch(new CakeEvent('View.beforeRender', $this, array($file)));
}
Expand Down

2 comments on commit b8320fd

@rchavik
Copy link
Member

@rchavik rchavik commented on b8320fd Sep 1, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we handle overidden helpers in helpers. For example, this change broke Croogo's test:

https://github.com/croogo/croogo/blob/master/Plugin/Croogo/Test/Case/View/Helper/CroogoFormHelperTest.php#L173

as it overrides Html helper implicitly, but then it reuses existing helper from the view.

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the view is loading the htmlhelper on construction. You could either remove the autoloading helpwer, or directly set the custom HTML helper into the collection. The second option is a bit shit though.

Please sign in to comment.