Skip to content
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

[5.3] Allow passing a Closure to View::share() method #15312

Merged
merged 2 commits into from
Sep 7, 2016
Merged

[5.3] Allow passing a Closure to View::share() method #15312

merged 2 commits into from
Sep 7, 2016

Conversation

memu
Copy link
Contributor

@memu memu commented Sep 6, 2016

This pull request allows you to pass a Closure as the $value parameter to View::share() method.

Example

class AppServiceProvider extends ServiceProvider
{
    public function boot()
    {
        View::share('key', function () {
            // do something
            return 'something else';
        });
    }
    //.....
}

Allows you to pass a Closure as the $value parameter to View::share() method.
@taylorotwell taylorotwell merged commit f971564 into laravel:5.3 Sep 7, 2016
@taylorotwell
Copy link
Member

Nice one.

@polonkaig
Copy link

Hi! This PR breaks my blade templates. I pass an array of closures to an @include-d subview. In that subview i did a foreach on that array, and after that i try to include an other subview. At that time, the second @include try to call my last closure in the array without parameters, and it throws an exception. If you want i can create a minimal example and create an issue ticket.

@tortuetorche
Copy link
Contributor

tortuetorche commented Sep 14, 2016

This pull request broke my Controller helpers:

<?php

namespace App\Http\Controllers;

use HTML;
use Illuminate\Foundation\Bus\DispatchesJobs;
use Illuminate\Routing\Controller as BaseController;
use Illuminate\Foundation\Validation\ValidatesRequests;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;

class Controller extends BaseController
{
    use AuthorizesRequests, DispatchesJobs, ValidatesRequests;

    protected $resource;

    public function __construct()
    {
        // Sortable helper for views
        view()->share('sortable', function ($column, $title = null, $options = []) {
            $defaultOptions = ['default' => false];
            $options = array_merge($defaultOptions, $options, compact('title'));
            $modelName = studly_case(
                array_pull(
                    $options,
                    'modelName',
                    array_pull($options, 'className', $this->getResourceName())
                )
            );
            return HTML::sortableLink($modelName, $column, $options);
        });
    }

    /**
     * Return the resource name, generally the model name attached to the controller.
     *
     *   UsersController -> user
     *   SuperUserController -> super_user
     *
     * @param string $controllerName Controller name. Default to null
     * @return string
     */
    protected function getResourceName($controllerName = null)
    {
        if (! $controllerName && $this->resource && is_string($this->resource)) {
            return str_singular($this->resource);// short circuit
        }

        // Otherwise guess the resource name with the controller name
        $controllerName = $controllerName ?: get_called_class();
        $name = preg_replace("/^(.+)Controller$/", "$1", $controllerName);

        return str_singular(snake_case(class_basename($name)));
    }
}

So in my views I could do this:

<table>
  <thead>
    <tr>
      <th>{!! $sortable('username') !!}</th>
      <th>{!! $sortable('name', 'Fullname') !!}</th>
      <th>{!! $sortable('email') !!}</th>
      <th>{!! $sortable('updated_at') !!}</th>
    </tr>
  </thead>
  {{-- ... --}}
</table>

But with this new feature, a FatalErrorException is thrown with this message: "Function name must be a string".

@polonkaig
Copy link

I think this feature should be handled in the share method, not in the gatherData.

@themsaid
Copy link
Member

@tortuetorche I wonder why not use a helper method instead of passing a closure like that?

@themsaid
Copy link
Member

@polonkaig handling it in the share method will cause the same effect on @tortuetorche way of passing a closure.

@tortuetorche
Copy link
Contributor

@tortuetorche I wonder why not use a helper method instead of passing a closure like that?

See the Controller::getResourceName() method, the className option of my helper is autofilled with it.

@tortuetorche
Copy link
Contributor

And this type of helpers are only available for the views, with isolation in mind.

@themsaid
Copy link
Member

I agree this change is breaking but not sure how to fix without removing the added functionality.

@crynobone
Copy link
Member

Shouldn't this be reverted due to breaking BC nature? It should instead be merged for 5.4 release

@themsaid
Copy link
Member

The thing is that the feature was released, was hoping to find a way to fix instead of completely reverting. Reverting will break the apps where developers made use of the new added functionality.

@taylorotwell
Copy link
Member

Reverted.

@rs-sliske
Copy link

is there a work around for getting this behavior to work again?

just updated to 5.3.10 and now my views are broken :(

@themsaid
Copy link
Member

@rs-sliske maybe call it as a method $variable()?

@rs-sliske
Copy link

@themsaid that seems to have worked thanks :)

@memu memu deleted the patch-1 branch January 12, 2017 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants