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

[8.x] Add middleware to View rendering #34339

Closed
wants to merge 1 commit into from

Conversation

browner12
Copy link
Contributor

@browner12 browner12 commented Sep 14, 2020

Please help me decide if I'm taking crazy pills with this one...

The motivation behind this PR is to be able to take the rendered content from a View, and then manipulate it, before passing it on. My use case for this is to handle so-called "shortcodes" in my output. So in a WYSIWYG the user could write something like [user id="1" field="first_name"], and we would replace that with User 1's first name.

Another use-case might be to censor swear words on your site.

We do this by hooking into the renderContents() method of the View.php, and running the generated content through a Pipeline. We use it by calling the middleware() method on a View instance.

$middleware = function($content, $next)
{
    $content = str_ireplace('facade', '', $content);

    return $next($content);
}

view('my/view')->middleware($middleware);

This can currently be handled with traditional middleware, but it feels a little off digging into the content of a Response to do this. Not sure how others feel.

public function handle($request, Closure $next)
{
    $response = $next($request);

    $content = $response->getContent();
    $content = str_ireplace('facade', '', $content);

    $response->setContent($content);

    return $response;
}

we pass the View rendered content through a pipeline, so we can manipulate the content using a middleware.
@taylorotwell
Copy link
Member

taylorotwell commented Sep 14, 2020

What would each view "middleware" receive? The raw string content of the rendered view?

@browner12
Copy link
Contributor Author

browner12 commented Sep 14, 2020

Yes, the rendered content from the appropriate engine, which would be a string.

The current Shortcode package that I use overrides the default Laravel View/View.php and View/Factory.php classes in order to accomplish hooking into the View rendering at this point, which can cause issues if changes are made in these files to core, that don't propagate to the package.

This was my attempt at having a built in way for us to hook into this point.

@ryangjchandler
Copy link
Contributor

ryangjchandler commented Sep 15, 2020

Is there any way that the use of the app() helper could be avoided, so that anybody using the illuminate/view package outside of a Laravel environment could still make use of this functionality with whatever Illuminate\Contracts\Container\Container implementation they are using?

@driesvints
Copy link
Member

driesvints commented Sep 15, 2020

@ryangjchandler could do Container::getInstance() which would be a bit better and not bind it to the framework but you'd still need to make use of the Laravel container that way. I'd suggest to do this change anyway regardless of the outcome of the PR.

@@ -119,7 +127,10 @@ protected function renderContents()

$this->factory->callComposer($this);

$contents = $this->getContents();
$contents = (new Pipeline(app()))
Copy link
Member

@driesvints driesvints Sep 15, 2020

Choose a reason for hiding this comment

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

Replace with:

Suggested change
$contents = (new Pipeline(app()))
$contents = (new Pipeline(Container::getInstance()))

@ryangjchandler
Copy link
Contributor

ryangjchandler commented Sep 15, 2020

@ryangjchandler could do Container::getInstance() which would be a bit better and not bind it to the framework but you'd still need to make use of the Laravel container that way. I'd suggest to do this change anyway regardless of the outcome of the PR.

Yeah, of course, it would need to extend from the base container that Laravel provides. More of a focus on not using support helper functions.

@driesvints
Copy link
Member

driesvints commented Sep 15, 2020

@ryangjchandler Just the interface, not the entire base container.

@ryangjchandler
Copy link
Contributor

ryangjchandler commented Sep 15, 2020

@ryangjchandler Just the interface, not the entire base container.

Yeah, that's what I meant - it's still early 😉

@taylorotwell
Copy link
Member

taylorotwell commented Sep 18, 2020

I wonder if this can be solved with just a more elegant method on Illuminate\Http\Response for modifying the content rather than introducing an entirely new middleware concept?

@browner12
Copy link
Contributor Author

browner12 commented Sep 18, 2020

I'm definitely open to other ideas. This solution didn't turn out as elegant as I had hoped.

@browner12 browner12 deleted the view-middleware branch Sep 18, 2020
@ksassnowski
Copy link
Contributor

ksassnowski commented Sep 22, 2020

It might be possible to use blade components for this. If you return a function from the render method you would be able to access $attributes['slot'] on it which is an instance of Illuminate\Support\HtmlString (see https://laravel.com/docs/8.x/blade#using-attributes-slots-inside-the-class). So maybe it would be possible to define something akin to a Vue renderless component?

<x-my-component>
   <p>Some string with [shortcode]</p>
</x-my-component>
class MyComponent extends Component
{
    public function render()
    {
        return function (array $attributes) {
            $contents = $attributes['slot'];

            return '<div>'.replaceShortCode($contents).'</div>';
        };
    }
}

Note that this is completely untested, however. I was reading through the docs and had to think of this issue.

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Sep 23, 2020

It would be good to have a solution bound to the View component and not just in the Response object, as views can be used standalone such as in Mailables. One frequent use-case I have is to use them through the barryvdh/laravel-dompdf package to generate PDFs.

So a solution not bound to the request/response would be handy.

I personally liked the pipeline/middleware approach. I started using job middleware recently and find the approach very good.

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.

None yet

6 participants