lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.49k stars 121 forks source link

Laravel 10 doesn't works expectsJson now is isJson #265

Closed n3storm closed 6 months ago

n3storm commented 9 months ago

/src/Decorators/ControllerDecorator.php at lines 73 and 75 should use isJson()

       if ($this->hasMethod('jsonResponse') && $request->expectsJson()) {
            $response = $this->callMethod('jsonResponse', [$response, $request]);
        } elseif ($this->hasMethod('htmlResponse') && ! $request->expectsJson()) {
            $response = $this->callMethod('htmlResponse', [$response, $request]);
        }

or check both for backwards compatibility:

        if ( $this->hasMethod('jsonResponse') && ($request->expectsJson() || $request->isJson() ) ) {
            $response = $this->callMethod('jsonResponse', [$response, $request]);
        } elseif ( $this->hasMethod('htmlResponse') && ! ($request->expectsJson() || $request->isJson() ) ) {
            $response = $this->callMethod('htmlResponse', [$response, $request]);
        }

Comment this issue if you want me to pull request and which option would you prefer.

lorisleiva commented 9 months ago

Hi there, thanks for raising this and offering to write a PR. Option B would be preferred as we are currently supporting both Laravel 9 and 10.

jameswagoner commented 9 months ago

That PR broke all my inertia requests.

I am not sure why this was needed. I am running L10 in production with Laravel Actions making of html and json response methods on the same action as expected

lorisleiva commented 9 months ago

@jameswagoner Ugh sorry about that. I'll revert this PR and publish a new patch version.

@n3storm Can you provide more information as to why that change was necessary for you?

n3storm commented 8 months ago

I am reviewing this issue now

n3storm commented 8 months ago

My bad sorry to you all @jameswagoner and @lorisleiva

Laravel expects an "Accept" header requesting JSON data, not Content-type, in order to response JSON

https://github.com/laravel/passport/issues/100#issuecomment-248917207

Is not a default header when using tools like Postman nor Insomnia and is wrongly documented with Content-type header at several internet resources.